-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: Fixed xfail for tests in pandas/tests/tseries/offsets/test_offsets_properties.py #32465
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
Conversation
@@ -85,8 +85,6 @@ | |||
# Offset-specific behaviour tests | |||
|
|||
|
|||
# Based on CI runs: Always passes on OSX, fails on Linux, sometimes on Windows | |||
@pytest.mark.xfail(strict=False, reason="inconsistent between OSs, Pythons") |
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.
It says here that on linux it always fails (at least this is how I understood it), but it passed on my machine (I run linux).
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 so I think I understood why this failing:
hypothesis.extra.dateutil.timezones
is using dateutil.UTC
which is avaible only from dateutils>=2.7.0
(https://dateutil.readthedocs.io/en/stable/tz.html#dateutil.tz.dateutil.tz.UTC)
but macOS have a pinned python-dateutils
to 2.6.1
which then leads to errors.
@jbrockmendel is bumping python-dateutils
to 2.7.0
is an option/reasonable thing to do?
if not I will put a pytest.mark.skipif
decorator (seems more likely to be the answer)
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.
is bumping python-dateutils to 2.7.0 is an option/reasonable thing to do?
2.7.0 is 2 days shy of 2 years old, so I think we're OK bumping
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.
@MomIsBestFriend this is definitely an improvement, but we can also just bump the dateutil requirement
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.
@MomIsBestFriend this is definitely an improvement, but we can also just bump the dateutil requirement
Sure! will do that in a separate PR
assert res[0] == rng[0] + offset | ||
assert res[-1] == rng[-1] + offset | ||
res2 = ser + offset | ||
# apply_index is only for indexes, not series, so no res2_v2 | ||
|
||
# TODO: Check randomly assorted entries, not just first/last |
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.
I am interested in doing that, anyone got a smart way of doing that?
Will do that in a follow-up PR.
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.
for _ in range(10):
n = np.random.randint(len(ser))
assert res2.iloc[n] == ser.iloc[n] + offset
(10 is arbitrary number here, you decide what is reasonable number
4ea120e
to
6112ddd
Compare
|
@@ -101,7 +103,7 @@ def test_on_offset_implementations(dt, offset): | |||
|
|||
|
|||
@pytest.mark.skipif( | |||
dateutil.__version__ < "2.7", | |||
DATEUTIL_VERSION < "2.7", | |||
reason="hypothesis uses dateutil.tz.UTC which was intreduced in dateutils 2.7.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.
intreduced -> introduced
@@ -101,7 +103,7 @@ def test_on_offset_implementations(dt, offset): | |||
|
|||
|
|||
@pytest.mark.skipif( | |||
dateutil.__version__ < "2.7", | |||
DATEUTIL_VERSION < "2.7", |
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.
can you use LooseVersion
looks like the build logs have expired; can you rebase and we'll see if this is ready |
|
||
# rng = pd.date_range(start='1/1/2000', periods=100000, freq='T') | ||
# TODO: Remove this `warnings.simplefilter` |
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.
what are the conditions that will allow this?
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.
I can't remebmer why was this necessery, Ill remove it, and see if the CI passes
Couple of comments, looks nearly there |
This reverts commit 635b28e.
|
Is this still active? |
closing as stale; if you can continue pls ping / open a new PR. |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff