-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
TST: remove patches to pandas.util.testing.N #24826
Conversation
Another solution might be to add this as a keyword argument to those functions, so the monkeypatch is not needed? |
Codecov Report
@@ Coverage Diff @@
## master #24826 +/- ##
=======================================
Coverage 92.38% 92.38%
=======================================
Files 166 166
Lines 52379 52379
=======================================
Hits 48392 48392
Misses 3987 3987
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24826 +/- ##
==========================================
- Coverage 92.38% 92.38% -0.01%
==========================================
Files 166 166
Lines 52379 52379
==========================================
- Hits 48392 48391 -1
- Misses 3987 3988 +1
Continue to review full report at Codecov.
|
are you not a fan of monkeypatch? there are only 4 instances where the patch is needed, one of those is for depecated Panel another instance is code duplication resulting from some tests using unittest setup method and other tests having the setup method converted to pytest fixtures. the monkeypatch used as context manager is to make it more explicit what is happening and ensuring that the patch in the fixture is undone before the test executes. this may appear to add additional lines of code. my only concern with altering pandas.util.testing is that it is in the pandas directory and not the tests directory, so a change here would be considered an api change? |
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.
see my comments; all of the examples should not need patching
"""DataFrame with 3 level MultiIndex (year, month, day) covering | ||
first 100 business days from 2000-01-01 with random data""" | ||
tm.N = 100 | ||
tdf = tm.makeTimeDataFrame() | ||
with monkeypatch.context() as m: |
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.
why do all this, makeTimeDataFrame(100)
will do this
looks ok, ping on green. |
@jreback green! |
Thanks @simonjayhawkins |
xref #24769 (comment)