FORMAT: __repr__ of Timestamp should include the offset (freq) info

[jreback]

In [6]: Timestamp('20130101',offset='D')
Out[6]: Timestamp('2013-01-01 00:00:00', tz=None)

In [7]: Timestamp('20130101',offset='D').freq
Out[7]: 'D'

[jtratner]

This would be trivial to do, right? Are you doing a PR for this? Could be a
good 'first PR' though

[jreback]

that's why I put it out there!

[jtratner]

:)

[jtratner]

@jesukumar go for it!

[rosnfeld]

I will take this if it's still available?

[jreback]

go 4 it

[cancan101]

See also: #6560 (comment)

[rosnfeld]

One thing I'll note here is that there is no validation of the offset/frequency, it's just copied into the object directly, you can put whatever you want:

In [20]: pd.Timestamp('20130101',offset={1:2, 5:6}).freq
Out[20]: {1: 2, 5: 6}

Not sure if we should worry about that, but basically as it stands we don't know what type of object we'd be processing in __repr__().

[rosnfeld]

Also, related to the comments on #6560 - where is the __str__ defined for Timestamp? It's clearly different from the __repr__ and yet I don't see it anywhere. I don't really know Cython so maybe there is some magic there..?

[jreback]

@rosnfeld I think you would have to write __str__ in the tslib.py/Timestamp class

it is currently defined using essentialy the datetime.datetime.__str__ (which is a c-method)

you can do this almost exactly like __repr__ is done

[rosnfeld]

So somehow the "_Timestamp" that Timestamp inherits from is related to datetime.datetime? Interesting, I'd be curious to learn how that works.

On #6560 you mention "a repr is something that can be copy pasted to reverse the object" - well, a simplistic implementation of just calling repr on the offset is going to do something like:

Timestamp('2014-03-05 00:00:00-0500', tz='US/Eastern', offset='<Day>')

whereas I think you are hoping to see the freqstr, as in

Timestamp('2014-03-05 00:00:00-0500', tz='US/Eastern', offset='D')

but neither of these are things that can be copy-pasted into the Timestamp constructor, as it expects a fully formed Offset class from tseries.offsets, unlike the Period constructor which can take a freqstr.

Should the Timestamp constructor handle more formats? That's starting to sound like a larger change.

[jreback]

This does seem to work

In [5]: Timestamp('2014-03-05 00:00:00-0500', tz='US/Eastern', offset='M').offset
Out[5]: 'M'

[rosnfeld]

Yeah, but that is literally just the string 'M', try doing +/- operations on that Timestamp.

Although, Timestamp.to_period() works in that case as Period is happy about receiving the string 'M'.

[rosnfeld]

Maybe the answer is really for Timestamp to store frequency internally as a string, as Period does. That will require some other changes though, as e.g. a Timestamp from a date_range() has an offset object.

[jreback]

ahh..so that's a bug then....no its easy to convert the offset if its a string

e.g.

if util.is_string_object(offset):
   from pandas.tseries.frequencies import to_offset
   offset = to_offset(offset)

but have to make sure this doesn't screw up perf (though usually a Timestamp is ONLY passed a string offset if you create it singly, so no big deal).

[rosnfeld]

Yeah, I have yet to find any examples of code passing the string offset. I assume util.is_string_object isn't an expensive test perf-wise, as its cost will be added to all instantiations?

[jreback]

the util.is_string_object is not expensive at all...its the actual offset creation that is not super duper cheap (and might be impactful if you are doing a lot)

[rosnfeld]

Yup, just checking to make sure I understood.

I'm starting to look at the str() implementation and I think it's going to be a bit more involved, and it's going to be a change a lot of people see (e.g. anyone who outputs a frequency-based timeseries). I think I'll create a new issue to track that as neither this issue nor PR #6560 really seem the natural place to discuss it.

I'll file a PR to get the code for this item in first.

[rosnfeld]

Well, after playing with the str() code a bit and trying to write the new issue I realized I am not in favor of the idea, as I noted in #6560 (comment) . Guess it's good I filed the PR separately.