Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[json schema] Do not throw errors when parsing non annotated parameters #1157

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Small fixes should be here.
## Features

## 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)


# v0.6.6 (December 20, 2024)
Expand All @@ -36,6 +37,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 @@ -50,7 +52,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 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)
* 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 = False, **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)
Loading