Skip to content

[REF] Move constructor helpers to EA Mixin classes #21843

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

Closed
wants to merge 1 commit into from

Conversation

jbrockmendel
Copy link
Member

After this:

  • Tests
  • Comparisons
  • Tests
  • Deal with the fact that DatetimeIndex caches ranges, only feasible because of immutability
  • Tests!
  • formatting/__repr__ methods
  • Teeeeeests
  • Double-check docstring accuracy

@jorisvandenbossche
Copy link
Member

Are there methods left that can rather simply be moved?

@jbrockmendel
Copy link
Member Author

Are there methods left that can rather simply be moved?

For the arithmetic/comparison methods (and their dependencies) that I've mostly been focused on, not really. Most of what's left either a) dispatches to the base Index class and so is non-trivial, or b) requires more of __new__ to be ported (DatetimeIndexOpsMixin.shift passes start and end kwargs to the constructor which are not currently handled).

There are a few more that can be moved with Index-wrapping left in the Index subclasses. DatetimeIndex.to_period, PeriodIndex.to_timestamp, ...

There are a few that would be simple but I'm not yet sure if they should be moved: inferred_type, is_all_dates, is_type_compatible, PeriodIndex.tz_convert, PeriodIndex.tz_localize

@jbrockmendel
Copy link
Member Author

Based on tests it looks like this is not the correct next step. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants