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

API: deprecate range-generating keywords in DatetimeIndex #20535

Closed
jorisvandenbossche opened this issue Mar 29, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@jorisvandenbossche
Copy link
Member

commented Mar 29, 2018

I was reviewing the PR on the DatetimeIndex docstring, and the list of parameters is horribly complex because you can both use it by passing data (with accompanying keywords like copy, dtype, ..) and by generating a range with optional start, end, periods.

Since for generating ranges, we have the special purpose pandas.date_range, do we need to keep this functionality in the DatetimeIndex constructor as well?

Of course deprecating it will annoy people who use it (although I think the majority of usage of DatetimeIndex passed data), so the question is whether it is worth it.

@mroeschke

This comment has been minimized.

Copy link
Member

commented Mar 29, 2018

+1. I'd be great to extend this to TimedeltaIndex and PeriodIndex as well.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2018

yep should do this

@jreback jreback added this to the Next Major Release milestone Mar 30, 2018

@jbrockmendel jbrockmendel added this to DTI/DTA Constructor Issues in DatetimeArray Refactor Nov 16, 2018

@jreback jreback modified the milestones: Contributions Welcome, 0.24.0 Nov 27, 2018

@jorisvandenbossche

This comment has been minimized.

Copy link
Member Author

commented Nov 28, 2018

We still need to do the same for PeriodIndex.

@jbrockmendel jbrockmendel moved this from DTI/DTA Constructor Issues to Done in DatetimeArray Refactor Dec 7, 2018

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

I'm working on this now. What's the plan for fields? I don't actually know what that does.

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

Hmm, slight API change (I think it counts as a bug) when start / end is a Period with a non-daily freq.

In [2]: start = pd.Period("2000", freq="B")

In [3]: pd.period_range(start, periods=4).freq
Out[3]: <Day>

In [4]: pd.PeriodIndex(start=start, periods=4).freq
Out[4]: <BusinessDay>

I think we want Out[4]. On master this is incorrect because the default value in period_range is 'D', not None.

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

On fields, I think that we should continue to accept them in the PeriodIndex constructor (at least, we shouldn't add them to period_range).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.