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

ENH: set __module__ for pandas scalars (Timestamp/Timedelta/Period) #57976

Merged

Conversation

YinonHorev
Copy link
Contributor

@YinonHorev YinonHorev commented Mar 23, 2024

Addresses part of #55178

@jorisvandenbossche
Copy link
Member

Thanks for the PR!

Can you add those new cases to the existing test that was added here: https://github.com/pandas-dev/pandas/pull/55171/files#diff-f2ce09edfcd4e105695b327053c3d288fa9547b1b5cb6488da3f3213e5478870?

Comment on lines 285 to 284
@set_module("pandas")
cdef class Interval(IntervalMixin):
Copy link
Member

Choose a reason for hiding this comment

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

For Interval, it seems this is not working (see the build failures) because this is a cdef class instead of just a class.

I don't directly know the best workaround, but so can also leave out Interval from this PR and already do the others

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK will do.

@YinonHorev YinonHorev force-pushed the set-__module__-scalars branch 2 times, most recently from 59f66a8 to 0545c4e Compare March 23, 2024 16:54
@jorisvandenbossche jorisvandenbossche added Sprints Sprint Pull Requests Output-Formatting __repr__ of pandas objects, to_string labels Mar 25, 2024
@jorisvandenbossche
Copy link
Member

There is another failing test that needs to be updated:

msg = "<class 'pandas._libs.tslibs.timestamps.Timestamp'>"
with pytest.raises(TypeError, match=msg):
timezones.maybe_get_tz(Timestamp("2021-01-01", tz="UTC"))

The expected msg needs to be updated now

Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Apr 25, 2024
@jorisvandenbossche jorisvandenbossche changed the title ENH: set __module__ for objects in pandas Scalars API ENH: set __module__ for objects in pandas Scalars API (Timestamp/Timedelta/Period) Nov 7, 2024
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Apologies for the slow follow-up, but rebased this once more and fixed the docstring failure, so all good to go now.

@YinonHorev thanks again for the PR!

@jorisvandenbossche jorisvandenbossche changed the title ENH: set __module__ for objects in pandas Scalars API (Timestamp/Timedelta/Period) ENH: set __module__ for pandas scalars (Timestamp/Timedelta/Period) Nov 7, 2024
@jorisvandenbossche jorisvandenbossche merged commit f9d2e50 into pandas-dev:main Nov 7, 2024
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Output-Formatting __repr__ of pandas objects, to_string Sprints Sprint Pull Requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants