-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
BUG: DatetimeIndex.insert doesnt preserve name and tz #7299
Conversation
did you test if adding a new date to a time-series with a given frequency then changes to |
@jreback Added a logic to check the item is on And should set |
I am pretty sure that insert will almost always reset the freq to None (I think it reinfers inside the constructor). just want to tests (and you example is not True, freq is
|
Sorry, for barging in, I could've sworn I've seen code like this elsewhere. I've finally remembered: it was the subclassing guide. in numpy docs. As long as |
OK. Based on Because timezone is corrupted if values are directly passed to
#7302 should also be fixed because |
@jreback I'm now in a rather deadline-ish mode just skimming through mail in spare time. Once I'm done with that, I could take a stab at it. |
@sinhrks yep I think that is correct; you basically DON't want to pass in a freq when you insert and let it reinfer (what I was saying is a) test this b) you can prob do a quick check if the inserted matches the same freq and would be faster so you CAN pass in a new freq). also, I think delete should work the same (so need to fix that too). maybe a function |
Edit: moving my comment to #7302 |
@jreback. Thanks. But I think checking whether the constructed DTI meets the freq in advance is little difficult in some cases. For example:
I've modified to call |
I think the constructor should infer it (I just don't remember exactly how/when this happens), and in fact you may not want to infer it (e.g. let it happen automatically if necessary). Which I think is the prior behavior. So maybe don't don't explicity infer at all (i was just thinking it would be faster/easier to do that). |
@sinhrks I was thinking that checking the edge cases only for both insert/delete makes sense (for delete its easy as it would be the same freq), insert if the new stamp obeys the current freq. otherwise just pass thru |
@jreback OK. I've modified, and hope to meet what you saying. I feel the check is very specific logic and not split to a separate function for now. |
that looks right |
ok, that's fine (but its actually the same issue), |
@sinhrks pls fix this up similarly to |
@jreback Because
|
BUG: DatetimeIndex.insert doesnt preserve name and tz
this is fine for now...in the future could extend it; maybe add an issue for 0.15? |
DatetimeIndex.insert
doesn't preservename
andtz
attributes. Also modifiedDatetimeIndex.asobject
to return an objectIndex
which has the same name as original to cover the case when the result is being objectIndex
.