-
Notifications
You must be signed in to change notification settings - Fork 95
Feature/partial First Pydantic Models for parameter validation #678
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
base: master
Are you sure you want to change the base?
Conversation
…if seen relativly
| return len(obj) | ||
| elif isinstance(obj, neo.core.spiketrainlist.SpikeTrainList): | ||
| return len(obj) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added getting length of SpikeTrainList, to check if it is empty or has a min_length.
|
|
||
| def validate_array_content(value, info, allowed_types: tuple, allow_none: bool, min_length: int, allowed_content_types: tuple, min_length_content: int = 0): | ||
| validate_type_length(value, info, allowed_types, allow_none, min_length) | ||
| hasContentLength = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If hasContentLength is not set to True in the loop, then it would have never been initialized.
| @classmethod | ||
| def validate_spiketrains(cls, v, info): | ||
| return fv.validate_spiketrains(v, info) | ||
| return fv.validate_spiketrains(v, info, min_length=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spiketrains is also allowed to be empty
| # Call function with validated data unpacked | ||
| return func(**validated.model_dump()) | ||
| return func(*args, **kwargs) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because no normalization happens in the Pydantic models. It is safer to just pass the original arguments
| if kwargs.pop("not_validate", False): | ||
| # skip validation, call inner function directly | ||
| return func(*args, **kwargs) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be able to call function without validation, if it does not work how it should or if the user knows he has valid data and therefore wants to save some calculation time.
| cv = scipy.stats.variation | ||
| @validate_with(PydanticCv) | ||
| def cv(*args, **kwargs): | ||
| return scipy.stats.variation(*args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the simplest way I found, to use a decorator for cv, but maybe it could be done without changing the previous code.
| # Tau max with no units | ||
| tau_max = 1 | ||
| self.assertRaises(ValueError, | ||
| self.assertRaises((ValueError, TypeError), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the Pydantic Model is more specific, it sometimes raises a different error type than the function, so the test should allow both types
| skip_validation = False | ||
|
|
||
| def deactivate_validation(): | ||
| skip_validation = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should also be able to disable the validation globally, so it does not get annoying, if you never want to use it.
…ate, so the api does not need to be changed and to simplify the decorator
| return decorator | ||
|
|
||
| def activate_validation(): | ||
| global skip_validation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made it global so it gets stored globally and therefore stores the value correctly
| (elephant.statistics.instantaneous_rate, PydanticInstantaneousRate, {"spiketrains": [], "sampling_period": 0.01 * pq.s}), | ||
| (elephant.statistics.optimal_kernel_bandwidth, PydanticOptimalKernelBandwidth, {"spiketimes": np.array([])}), | ||
| (elephant.statistics.cv2, PydanticCv2, {"time_intervals": np.array([])*pq.s}), | ||
| ]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to pass in sampling_period into instantaneous_rate
Uh oh!
There was an error while loading. Please reload this page.