API: DatetimeIndex.set_freq

[TomAugspurger]

leftover from #20772

IIUC, the proposal is for datetime_index.set_freq(new_freq) to be an alternative to datetimeindex.freq = new_freq (please correct me if I'm wrong).

This would also apply to TimedeltaIndex.

[jschendel]

I think this should also include deprecating setting the .freq attribute, unless I'm misinterpreting #20772 (comment).

I've prepared some code to implement these changes in case the decision was made to add this in #20772, as I didn't want to be the person holding up 0.23.0. Just have some minor renaming changes to make (asfreq --> set_freq) before opening a PR.

[jreback]

yes that would be nice @jschendel

[jorisvandenbossche]

To reiterate my reason to hold back on this on the previous PR:

• This is not a common operation, but, we want that is is possible for those that need it. But given that, is this worth adding a new method? (since it is already possible)
• Another argument was that setting an attribute directly on an immutable object is "wrong". I would argue that this is rather pythonic to work with getter/setter properties instead of get_, set_ functions (although in this case we only have the set_ part).
  Also, this does not alter the underlying Index whatsoever, it is just changing a single informative attribute, just like we can do idx.name = 'val'
• Allowing setting freq on a DatetimeIndex/TimedeltaIndex and not on PeriodIndex (that was deprecated in the previous PR) creates an inconsistency. This is true, however: a) the freq argument is really something of different importance for PI vs DTI/TDI, although it has the same name (for PI it is part of the dtype, and so setting it changes the interpretation of the data) and b) adding a set_freq to DTI/TDI also does not solve this inconsistency at all.

So to summarize: I just don't see the need to have this, doing dti.freq = 'D' already gives us what is needed.

[jreback]

@jorisvandenbossche not having this is just inconsistent. We have an immutable index, having any writebable properties is just plain wrong.

[jorisvandenbossche]

not having this is just inconsistent.

Did you read my explanation of why this does not solve inconsistency at all?

We have an immutable index, having any writebable properties is just plain wrong.

Please explain why this is "plain wrong", I give a whole bunch of arguments, you just "plain wrong". It's not because the index values are immutable that certain of its attributes cannot be mutable (name ?)

Personally I don't really care about this (it is such a minor use-case, but exactly for that I don't think it is worth a new method), but it annoys me you dismiss my attempt to discussion with just "plain wrong".

[jreback]

because setting an attribute in an immutable object is wrong - not sure how much more clear than that

for names we have set_name which returns a new object

one does not expect immutable objects to be mutated

[jorisvandenbossche]

for names we have set_name which returns a new object

Yes, but we also can set the name object directly. This works fine. And personally, if I need this, I use that instead of the set_names method:

idx = idx.set_names('name')
vs
idx.name = 'name'

[TomAugspurger]

We set the name directly in several places in the docs as well. What's the (user-facing) issue caused by that?

…

On Tue, May 1, 2018 at 7:40 AM, Joris Van den Bossche < ***@***.***> wrote: for names we have set_name which returns a new object Yes, but we also can set the name object directly. This works fine. And personally, if I need this, I use that instead of the set_names method: idx = idx.set_names('name') vsidx.name = 'name' — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20886 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIlblT3wpgnxHcgIuyg_ACBQSKQmtks5tuFe5gaJpZM4Ts93t> .

[jreback]

setting attributes on immutable objects is just a bad pattern. there is no reason to extend / expand this. we have been pro-actively removing this attribute access, e.g. .tz, .labels, and .levels for MI for a while. Why allow more of this when its not our idiomatic pattern. Sure .name is probably not ever changeable, but expanding this is a no-go.

Finally, having methods allows one to return a new object and chain things.

Setting attributes generallly is pretty much an anti-pattern, similar to the inplace kwarg (which we have slated for deprecation as well).

[jorisvandenbossche]

we have been pro-actively removing this attribute access, e.g. .tz, .labels, and .levels for MI for a while.

But all those examples do change the representation of the index data or change the index data itself. Setting the freq does not do such a thing.

Having a property (with getter/setter functionality under the hood) is certainly pythonic compared to explicit get_/set_ functions.

There are two reasons to have a set_freq method:

• To be able to use it in a method chain.
• Because we find that changing the freq of an index should always result in a new object (practical version of your immutability argument)

For the second bullet point: I personally don't think it is needed (it is in any case not needed technically), similarly as changing the name does not need to return a new object (so you can do eg df.index.name = ...).

And for the first, the question is then the question we always ask ourselves if somebody opens an issue for a new method / functionality: is this functionality worth it for adding yet another method to the already crowded namespace?
And in my opinion being able to chain it is not worth it.