-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
Fix to_yaml serialization dropping global checks #428
Merged
cosmicBboy
merged 22 commits into
unionai-oss:dev
from
antonl:bugfix/#419-to-yaml-drops-global-checks
Mar 23, 2021
Merged
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
3e2fad0
increase cache version
cosmicBboy b3ec21b
ci: add dataframe checks tests
antonl 49db091
bugfix: allow serialization of dataframe checks to_yaml
antonl c444d29
ci: add test to ensure serialization of lambda check fails
antonl 4f77a0d
bugfix: ensure checks with no parameters generate appropriate schema
antonl e23d6ed
wip: allow looking up registered checks
antonl e785344
fix: compare checks by name rather than by object equality
antonl ecbfaa6
ci: black
antonl 6f1df54
ci: lint
antonl c3b61ba
enh: use REGISTERED_CUSTOM_CHECKS for attribute lookup, add dir method
antonl a1a51fd
enh: add to_yaml method to Schema, add unit test
antonl 8f0790c
Merge remote-tracking branch 'upstream/dev' into bugfix/#419-to-yaml-…
antonl 92dc2f0
ci: disable typechecking on _CheckMeta
antonl d3a9b1b
ci: isort
antonl cbfaa98
ci: doctests
antonl ebaf219
ci: improve coverage
antonl 87438ad
ci: codecov
antonl 90d6871
fix unrecognized check dtype during (de)serialization
c067b7f
fix handle_stat__dtype closures
c3108b9
enh: move metaclass onto _CheckBase
antonl 0b2c453
ci: add test that ensures to_yaml warns on unregistered checks
antonl e1ee5d5
ci: revert adding duplicate test
antonl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
"""Pytest fixtures for testing custom checks.""" | ||
import unittest.mock as mock | ||
|
||
import pandas as pd | ||
import pytest | ||
|
||
import pandera as pa | ||
import pandera.extensions as pa_ext | ||
|
||
__all__ = "custom_check_teardown", "extra_registered_checks" | ||
|
||
|
||
@pytest.fixture(scope="function") | ||
def custom_check_teardown(): | ||
"""Remove all custom checks after execution of each pytest function.""" | ||
yield | ||
for check_name in list(pa.Check.REGISTERED_CUSTOM_CHECKS): | ||
del pa.Check.REGISTERED_CUSTOM_CHECKS[check_name] | ||
|
||
|
||
@pytest.fixture(scope="function") | ||
def extra_registered_checks(): | ||
"""temporarily registers custom checks onto the Check class""" | ||
# pylint: disable=unused-variable | ||
with mock.patch( | ||
"pandera.Check.REGISTERED_CUSTOM_CHECKS", new_callable=dict | ||
): | ||
# register custom checks here | ||
@pa_ext.register_check_method() | ||
def no_param_check(_: pd.DataFrame) -> bool: | ||
return True | ||
|
||
yield |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
"""Registers fixtures for core""" | ||
|
||
# pylint: disable=unused-import | ||
from .checks_fixtures import custom_check_teardown, extra_registered_checks |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Could you elaborate why it'd be better in
CheckBase
?REGISTERED_CUSTOM_CHECKS
is referenced by__getattr__
, it seems to make sense to have them sitting next to each others.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 comment got moved by black. Essentially, I think we could merge the
_CheckMeta
with_CheckBase
. The metaclass actually confusesmypy
, and I don't see the benefit of using this mixin pattern here. Since this is a style choice, I didn't want to just do it though, so I added a comment so that we could discuss.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 you move
__getattr__
to_CheckBase
it's going to act on instances of the class and not on the class itself.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.
Right. Metaclasses confuse me, had to look up the descriptor protocol docs again. 👍
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.
Ok, I'm proposing we just move the metaclass to
_CheckBase
. It's not usable on anything else anyway because it requires a name property for example. I'll fix it and remove the fixme comment.