DatetimeIndex::inferred_freq is misleading

[cancan101]

Looking at these three methods on DatetimeIndex:

    # alias to offset
    @property
    def freq(self):
        return self.offset

    @cache_readonly
    def inferred_freq(self):
        try:
            return infer_freq(self)
        except ValueError:
            return None

    @property
    def freqstr(self):
        return self.offset.freqstr

you would think that freqstr returns a string and inferred_freq and freq return "frequencies" (i.e. offsets). In reality inferred_freq is a string as well.

I am not sure of the best way to deal with this issue. At the very least I would suggest adding a inferred_freq_offset property. Perhaps also deprecate inferred_freq and create a inferred_freqstr.

Tracing this method, you find _FrequencyInferer::get_freq which returns a string rather than an Offset.

[cpcloud]

This is used a bit throughout the codebase (though not too much, grin shows 32 uses). Deprecation of inferred_freq is out of the question since the attribute is exposed publicly, and has been around since v0.8.0. I understand where you're coming from, but there's just too much legacy code that deprecating this could potentially break. Out of curiosity, do you find yourself using this property very often?

[cancan101]

@cpcloud Yeah, I did not think deprecation was realistic. I am more interested in just adding inferred_freq_offset.

I am looking to move away from passing around strings to represent DateOffsets and Periods and actually pass the objects themselves. This avoids having to keep performing "silly" map lookups (and it reduces a bunch of code, e.g. #5028).

The code that I am looking at that would benefit from inferred_freq_offset is: Series::to_period:

freq = self.index.freqstr or self.index.inferred_freq
new_index = self.index.to_period(freq=freq)

which I would like to change to:

freq = self.index.freq or self.index.inferred_freq_offset

allowing me to pass the freq object (rather than string) down to the next layer.

[nehalecky]

+1 on inferred_freq_offest.

[jtratner]

@cancan101 why not:

freq = getattr(self.index, 'offset', self.index.freq)

[cancan101]

@jtratner how does that help with inferred freq?

[jtratner]

@cancan101 sorry, I misread what you were trying to do. If you are going to change the internals to avoid the lookups for date ranges, why not just have inferred_freq return DateOffset?

[jtratner]

And then in the equality method for DateOffset, just let it return True if it matches its own rule code.

[cpcloud]

hmmm ... inferred_freq is sort of public-ish ....

[jtratner]

Yeah, but unless you think people are trying to do string concatenation/multiplication with it, having __str__ and __unicode__ return the rule code and __eq__ and __req__ return True if rule code matches makes it relatively transparent.

[cancan101]

@jtratner

Two issues:

1. What @cpcloud said
2. If I change eq I would have to change hash etc as well. Not end of world ,just have to think of implications

[jtratner]

@cancan101 well, if freq is exactly as expressive as offset, then you can just have the hash be the rule code right?

[jtratner]

and by rule code I mean what you pass to period index/datetime index for freq

[cancan101]

No. There is many to one mapping string to offset.

[jtratner]

@cancan101 well, you could pick one mapping to be the actual representation, hash however you want (doesn't really matter so long as it's consistent) and then have __eq__ look through all the aliases too.

[jtratner]

As an aside, __hash__ doesn't matter that much, because python still does an equality check on lookups after looking up via hash.