From aa060de05c12b7d2b9542aa7697d8c44825990b4 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Thu, 16 Jan 2025 14:25:40 -0600 Subject: [PATCH] Improve error message on `get_json_schema_from_method_signature` when passing parameters without type hints (#1157) --- CHANGELOG.md | 6 ++++-- .../ecephys/basesortingextractorinterface.py | 2 +- src/neuroconv/utils/json_schema.py | 10 +++++++--- ...est_get_json_schema_from_method_signature.py | 17 +++++++++++++++++ 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77b932e6d..3391f828c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,6 @@ ## Bug Fixes ## Features -* Source validation is no longer performed when initializing interfaces or converters [PR #1168](https://github.com/catalystneuro/neuroconv/pull/1168) ## Improvements @@ -21,9 +20,11 @@ Small fixes should be here. * Add description to inter-sample-shift for `SpikeGLXRecordingInterface` [PR #1177](https://github.com/catalystneuro/neuroconv/pull/1177) ## Improvements +* `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) * Improve the naming of ElectrodeGroups in the `SpikeGLXRecordingInterface` when multi probes are present [PR #1177](https://github.com/catalystneuro/neuroconv/pull/1177) * Detect mismatch errors between group and group names when writing ElectrodeGroups [PR #1165](https://github.com/catalystneuro/neuroconv/pull/1165) * Fix metadata bug in `IntanRecordingInterface` where extra devices were added incorrectly if the recording contained multiple electrode groups or names [#1166](https://github.com/catalystneuro/neuroconv/pull/1166) +* Source validation is no longer performed when initializing interfaces or converters [PR #1168](https://github.com/catalystneuro/neuroconv/pull/1168) # v0.6.6 (December 20, 2024) @@ -42,6 +43,7 @@ Small fixes should be here. * `SpikeGLXNIDQInterface` is no longer written as an ElectricalSeries [#1152](https://github.com/catalystneuro/neuroconv/pull/1152) * Fix a bug on ecephys interfaces where extra electrode group and devices were written if the property of the "group_name" was set in the recording extractor [#1164](https://github.com/catalystneuro/neuroconv/pull/1164) + ## 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) * Imaging interfaces have a new conversion option `always_write_timestamps` that can be used to force writing timestamps even if neuroconv's heuristics indicates regular sampling rate [PR #1125](https://github.com/catalystneuro/neuroconv/pull/1125) @@ -56,7 +58,7 @@ Small fixes should be here. * 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 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) * Enhance the typing of the signature on the `NWBConverter` by adding zarr as a literal option on the backend and backend configuration [PR #1160](https://github.com/catalystneuro/neuroconv/pull/1160) diff --git a/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py b/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py index 8eeb59324..9dfd8c82e 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 = True, **source_data): super().__init__(**source_data) self.sorting_extractor = self.get_extractor()(**source_data) self.verbose = verbose diff --git a/src/neuroconv/utils/json_schema.py b/src/neuroconv/utils/json_schema.py index 6aa7a75d0..b05019fe4 100644 --- a/src/neuroconv/utils/json_schema.py +++ b/src/neuroconv/utils/json_schema.py @@ -140,12 +140,16 @@ def get_json_schema_from_method_signature(method: Callable, exclude: Optional[li additional_properties = True continue - annotation = 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 e6fe27e16..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 @@ -424,3 +424,20 @@ def test_class_method(self, integer: int): } assert test_json_schema == expected_json_schema + + +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 test_method(param_with_type: int, param_without_type, param_with_default="default_value"): + pass + + 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)