Skip to content

Commit

Permalink
Improve error message on get_json_schema_from_method_signature when…
Browse files Browse the repository at this point in the history
… passing parameters without type hints (#1157)
  • Loading branch information
h-mayorquin authored Jan 16, 2025
1 parent 32f6834 commit aa060de
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 6 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions src/neuroconv/utils/json_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

0 comments on commit aa060de

Please sign in to comment.