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

Creates the validators registry #123

Merged
merged 10 commits into from
Oct 24, 2024
Merged

Creates the validators registry #123

merged 10 commits into from
Oct 24, 2024

Conversation

dalonsoa
Copy link
Collaborator

Additionally, run the validators when reading the header, converting the relevant fields into a pydantic object, and dump the data into dictionaries from the pydantic models when writing the data.

Close #79

Copy link
Contributor

@alexdewar alexdewar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know pydantic super well, but this LGTM!

VALIDATORS_REGISTRY[name] = cls
return cls

return decorator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd usually use the decorator package for this to simplify the syntax a little: https://pypi.org/project/decorator/

But this is totally fine as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this works well with classes, in particular in this case where we are just registering the class using an input to the decorator itself, not the function. So skipping, for now.

mock_write_csv.assert_not_called()

KNOWN_WRITERS.clear()
KNOWN_WRITERS.append(MagicMock(return_value=False))
write_data(filename, data, comment, **csv_options)
KNOWN_WRITERS[0].assert_called_once_with(filename, data, comment, **csv_options)
KNOWN_WRITERS[0].assert_called_once_with( # type: ignore [attr-defined]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also get mypy to ignore the tests folder if you can't be bothered with all these type: ignores.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has caught a few issues, so I rather not exclude the tests and just ignore the problematic lines.

@dalonsoa dalonsoa merged commit e71bad9 into develop Oct 24, 2024
8 checks passed
@dalonsoa dalonsoa deleted the registry branch October 24, 2024 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement validators registry
2 participants