From a90f295379d4323c5f6b6990a7569bba1958f3e0 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Fri, 13 Dec 2024 08:28:09 -0600 Subject: [PATCH 1/5] small addition --- .../datainterfaces/ecephys/basesortingextractorinterface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py b/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py index 8eeb59324..89acf6ed4 100644 --- a/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py +++ b/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py @@ -18,7 +18,7 @@ class BaseSortingExtractorInterface(BaseExtractorInterface): ExtractorModuleName = "spikeinterface.extractors" - def __init__(self, verbose=True, **source_data): + def __init__(self, verbose: bool = False, **source_data): super().__init__(**source_data) self.sorting_extractor = self.get_extractor()(**source_data) self.verbose = verbose From 508c8ea291a83d96f9e0de74f02ee1473a474163 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Fri, 13 Dec 2024 09:34:34 -0600 Subject: [PATCH 2/5] add test and fix --- src/neuroconv/utils/json_schema.py | 3 ++- ...t_get_json_schema_from_method_signature.py | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/neuroconv/utils/json_schema.py b/src/neuroconv/utils/json_schema.py index 6aa7a75d0..a7e6e0518 100644 --- a/src/neuroconv/utils/json_schema.py +++ b/src/neuroconv/utils/json_schema.py @@ -140,7 +140,8 @@ def get_json_schema_from_method_signature(method: Callable, exclude: Optional[li additional_properties = True continue - annotation = parameter.annotation + # If no annotation is provided (inspect._empty), use Any instead + annotation = Any if parameter.annotation is inspect._empty else parameter.annotation # Pydantic uses ellipsis for required pydantic_default = ... if parameter.default is inspect._empty else parameter.default diff --git a/tests/test_minimal/test_utils/test_get_json_schema_from_method_signature.py b/tests/test_minimal/test_utils/test_get_json_schema_from_method_signature.py index e6fe27e16..c2cea1d75 100644 --- a/tests/test_minimal/test_utils/test_get_json_schema_from_method_signature.py +++ b/tests/test_minimal/test_utils/test_get_json_schema_from_method_signature.py @@ -424,3 +424,26 @@ def test_class_method(self, integer: int): } assert test_json_schema == expected_json_schema + + +def test_get_json_schema_from_method_signature_unannotated(): + """Test that parameters without type annotations are handled correctly by defaulting to Any.""" + + def method_with_unannotated_params( + annotated_param: int, unannotated_param, unannotated_with_default="default_value" + ): + pass + + test_json_schema = get_json_schema_from_method_signature(method=method_with_unannotated_params) + expected_json_schema = { + "additionalProperties": False, + "properties": { + "annotated_param": {"type": "integer"}, + "unannotated_param": {}, # Any type in JSON Schema is represented by an empty object + "unannotated_with_default": {"default": "default_value"}, + }, + "required": ["annotated_param", "unannotated_param"], + "type": "object", + } + + assert test_json_schema == expected_json_schema From 540dadb9a9ac6b0173480173ca2d114420c7f921 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Fri, 13 Dec 2024 09:51:45 -0600 Subject: [PATCH 3/5] changelog --- CHANGELOG.md | 2 +- src/neuroconv/utils/json_schema.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11dce4e7a..b5fd6dc14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ * Fix a bug where data in `DeepLabCutInterface` failed to write when `ndx-pose` was not imported. [#1144](https://github.com/catalystneuro/neuroconv/pull/1144) * `SpikeGLXConverterPipe` converter now accepts multi-probe structures with multi-trigger and does not assume a specific folder structure [#1150](https://github.com/catalystneuro/neuroconv/pull/1150) * `SpikeGLXNIDQInterface` is no longer written as an ElectricalSeries [#1152](https://github.com/catalystneuro/neuroconv/pull/1152) - +* Avoid throwing validation errors when the signature of an interface does not annotate parameters [#1157](https://github.com/catalystneuro/neuroconv/pull/1157) ## Features * Propagate the `unit_electrode_indices` argument from the spikeinterface tools to `BaseSortingExtractorInterface`. This allows users to map units to the electrode table when adding sorting data [PR #1124](https://github.com/catalystneuro/neuroconv/pull/1124) diff --git a/src/neuroconv/utils/json_schema.py b/src/neuroconv/utils/json_schema.py index a7e6e0518..902f99e7d 100644 --- a/src/neuroconv/utils/json_schema.py +++ b/src/neuroconv/utils/json_schema.py @@ -142,6 +142,7 @@ def get_json_schema_from_method_signature(method: Callable, exclude: Optional[li # If no annotation is provided (inspect._empty), use Any instead annotation = Any if parameter.annotation is inspect._empty else parameter.annotation + annotation = parameter.annotation # Pydantic uses ellipsis for required pydantic_default = ... if parameter.default is inspect._empty else parameter.default From c1cb9606cfc41a00fc8d45718a5ff1ef3f46cd6a Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Fri, 13 Dec 2024 11:33:01 -0600 Subject: [PATCH 4/5] remove extra line for debugging --- src/neuroconv/utils/json_schema.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/neuroconv/utils/json_schema.py b/src/neuroconv/utils/json_schema.py index 902f99e7d..a7e6e0518 100644 --- a/src/neuroconv/utils/json_schema.py +++ b/src/neuroconv/utils/json_schema.py @@ -142,7 +142,6 @@ def get_json_schema_from_method_signature(method: Callable, exclude: Optional[li # If no annotation is provided (inspect._empty), use Any instead annotation = Any if parameter.annotation is inspect._empty else parameter.annotation - annotation = parameter.annotation # Pydantic uses ellipsis for required pydantic_default = ... if parameter.default is inspect._empty else parameter.default From e3c107d755916f8609bb73f81d73ebfe41bd4730 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Thu, 19 Dec 2024 08:52:39 -0600 Subject: [PATCH 5/5] change from omission to error --- CHANGELOG.md | 5 ++-- src/neuroconv/utils/json_schema.py | 11 ++++--- ...t_get_json_schema_from_method_signature.py | 30 ++++++++----------- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9193e5a8f..6f72b5faa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ * Fix a bug where data in `DeepLabCutInterface` failed to write when `ndx-pose` was not imported. [#1144](https://github.com/catalystneuro/neuroconv/pull/1144) * `SpikeGLXConverterPipe` converter now accepts multi-probe structures with multi-trigger and does not assume a specific folder structure [#1150](https://github.com/catalystneuro/neuroconv/pull/1150) * `SpikeGLXNIDQInterface` is no longer written as an ElectricalSeries [#1152](https://github.com/catalystneuro/neuroconv/pull/1152) -* Avoid throwing validation errors when the signature of an interface does not annotate parameters [#1157](https://github.com/catalystneuro/neuroconv/pull/1157) + ## Features * Propagate the `unit_electrode_indices` argument from the spikeinterface tools to `BaseSortingExtractorInterface`. This allows users to map units to the electrode table when adding sorting data [PR #1124](https://github.com/catalystneuro/neuroconv/pull/1124) @@ -29,7 +29,8 @@ * Use mixing tests for ecephy's mocks [PR #1136](https://github.com/catalystneuro/neuroconv/pull/1136) * Use pytest format for dandi tests to avoid window permission error on teardown [PR #1151](https://github.com/catalystneuro/neuroconv/pull/1151) * Added many docstrings for public functions [PR #1063](https://github.com/catalystneuro/neuroconv/pull/1063) -* Clean up with warnings and deprecations in the testing framework [PR #1158](https://github.com/catalystneuro/neuroconv/pull/1158) +* Clean up warnings and deprecations in the testing framework for the ecephys pipeline [PR #1158](https://github.com/catalystneuro/neuroconv/pull/1158) +* `get_json_schema_from_method_signature` now throws a more informative error when an untyped parameter is passed [#1157](https://github.com/catalystneuro/neuroconv/pull/1157) # v0.6.5 (November 1, 2024) diff --git a/src/neuroconv/utils/json_schema.py b/src/neuroconv/utils/json_schema.py index a7e6e0518..b05019fe4 100644 --- a/src/neuroconv/utils/json_schema.py +++ b/src/neuroconv/utils/json_schema.py @@ -140,13 +140,16 @@ def get_json_schema_from_method_signature(method: Callable, exclude: Optional[li additional_properties = True continue - # If no annotation is provided (inspect._empty), use Any instead - annotation = Any if parameter.annotation is inspect._empty else parameter.annotation + # Raise error if the type annotation is missing as a json schema cannot be generated in that case + if parameter.annotation is inspect._empty: + raise TypeError( + f"Parameter '{argument_name}' in method '{method_display}' is missing a type annotation. " + f"Either add a type annotation for '{argument_name}' or add it to the exclude list." + ) # Pydantic uses ellipsis for required pydantic_default = ... if parameter.default is inspect._empty else parameter.default - - arguments_to_annotations.update({argument_name: (annotation, pydantic_default)}) + arguments_to_annotations.update({argument_name: (parameter.annotation, pydantic_default)}) # The ConfigDict is required to support custom types like NumPy arrays model = pydantic.create_model( diff --git a/tests/test_minimal/test_utils/test_get_json_schema_from_method_signature.py b/tests/test_minimal/test_utils/test_get_json_schema_from_method_signature.py index c2cea1d75..111053797 100644 --- a/tests/test_minimal/test_utils/test_get_json_schema_from_method_signature.py +++ b/tests/test_minimal/test_utils/test_get_json_schema_from_method_signature.py @@ -426,24 +426,18 @@ def test_class_method(self, integer: int): assert test_json_schema == expected_json_schema -def test_get_json_schema_from_method_signature_unannotated(): - """Test that parameters without type annotations are handled correctly by defaulting to Any.""" +def test_json_schema_raises_error_for_missing_type_annotations(): + """Test that attempting to generate a JSON schema for a method with missing type annotations raises a TypeError.""" + # https://github.com/catalystneuro/neuroconv/pull/1157 - def method_with_unannotated_params( - annotated_param: int, unannotated_param, unannotated_with_default="default_value" - ): + def test_method(param_with_type: int, param_without_type, param_with_default="default_value"): pass - test_json_schema = get_json_schema_from_method_signature(method=method_with_unannotated_params) - expected_json_schema = { - "additionalProperties": False, - "properties": { - "annotated_param": {"type": "integer"}, - "unannotated_param": {}, # Any type in JSON Schema is represented by an empty object - "unannotated_with_default": {"default": "default_value"}, - }, - "required": ["annotated_param", "unannotated_param"], - "type": "object", - } - - assert test_json_schema == expected_json_schema + with pytest.raises( + TypeError, + match=( + "Parameter 'param_without_type' in method 'test_method' is missing a type annotation. " + "Either add a type annotation for 'param_without_type' or add it to the exclude list." + ), + ): + get_json_schema_from_method_signature(method=test_method)