-
-
Notifications
You must be signed in to change notification settings - Fork 150
GH1379 Drop OffsetSeries replacing it with Series[BaseOffset] #1390
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
pyproject.toml
Outdated
SQLAlchemy = ">=2.0.39" | ||
types-python-dateutil = ">=2.8.19" | ||
beautifulsoup4 = "<=4.13.5" | ||
beautifulsoup4 = ">=4.13.5" |
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.
beautifulsoup4 = ">=4.13.5" | |
beautifulsoup4 = ">=4.14.2" |
tests/test_timefuncs.py
Outdated
o_s = cast( | ||
"OffsetSeries", pd.Series([pd.DateOffset(days=1), pd.DateOffset(days=2)]) | ||
"pd.Series[BaseOffset]", | ||
pd.Series([pd.DateOffset(days=1), pd.DateOffset(days=2)]), | ||
) |
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 see if we can create a Series.__new__()
overload that would accept a sequence of BaseOffset
so that we don't need the cast here?
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.
So turns out this creates a Series[DateOffset]
and not Series[BaseOffset]
that is why this is an issue, not sure why though it is not working since it is supposed to inherit from BaseOffset.
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.
So I checked and seems like Series[DateOffset]
does not match with Series[BaseOffset]
even though DateOffset
is always a BaseOffset
:
assert_type(o_s, "pd.Series[BaseOffset]") ■ "assert_type" mismatch: expected "Series[BaseOffset]" but received "Series[DateOffset]"
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 think you need to add an overload for Series.__new__()
that accepts a sequence of BaseOffset
and then it should work. Similar to how there are overloads that produce Series[Timestamp]
, Series[Timedelta]
, etc.
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.
Removed the to_numpy overloads and added the new overloads and it is looking good!
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.
Thanks @loicdiridollou
assert_type()
to assert the type of any return valueAlso dropped the locking of the version of
beautifulsoup
, it seems like they shipped another version that fixed the issue.