-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: support for nanosecond time in offset and period #2555
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
Conversation
I get a build error with the |
What's the reason for requiring numpy >=1.7? |
And why can't I build with bleeding edge numpy? The old_defines.h header should not be included with git master numpy. I'll check this out. |
merging #2670 before should be possible. The most collisions are to be expected for period.c but they should be manageable. Only the "int64_t as i8" import introduces a lot of changes which may lead to conflicts. |
The reason for requiring numpy >= 1.7 is the support for nanoseconds in np.datetime64 which was not there before 1.7 |
Maybe its a good idea to remove format only changes in both PRs to ease merging. |
Branch did not build with bleeding edge numpy because of |
Will this fail gracefully (or can be made to) if a user has numpy 1.6 installed? |
You're right, for up to ms resolution numpy 1.6 should be enough. I'm not shure what it does at the moment, but I will try to make it as compatible as possible. |
Great, that would make it easier to get in. I see you merged upstream into your branch (rather then rebasing), cpcloud can attest to |
Hope it is still possible to switch to rebasing. You are welcome! |
@wuan, The last upstream merge you made was against a8abf1f. try this:
at this point, you're in the same state as if you were to start with a checkout of a8abf1f, and
Now you need to break these up into individual commits as best you can. Of course, you can break things up even further - it's your code, you know best what makes sense.
will let you interactively choose chunks to add to the index, if you want to go that route. Once that's done, you'll have a neat set of patches based on a8abf1f which Following that, mitigating the 1.7 hard dependency is important, and it'll be much easier |
pushed to 0.12, still need to resolve numpy 1.7 dep issue . |
Sorry, my changes were absent during your merge as I was just working on the transition to rebase. |
Should I open a new pull request referencing this one, or is it possible to reopen the request? My branch is now rebased against the pydata master. Nanoseconds will not work in numpy < 1.7, so some test cases should be skipped in that case. This is still work in progress. |
Wait did I miss something was this ready to merge? |
As far as I remember we wanted to merge #2670 before. And at least the numpy 1.6 compatibility should be added. |
@jreback don't make me take away your push rights =P |
shoot, sorry...I didn't mean to merge this one....similary named to another by stephenwlin... |
Can someone let me know if this is salvagable? |
unless I am reading somewrong, nothing was actually merged here? |
most of the changes were not there when the merge was done as I was transforming using rebase. I will have a closer look. |
This is my attempt to fix the so far missing nanosecond support for pd.DatetimeIndex (see issue #1812).
The Python modification should be relatively easy to review, but unfortunately I had to do a bigger refactoring of period.c in order to support subsecond times.
Im still working on adding more tests and I will try to include the tests on C level as well (test_period.c, which can only be used standalone so far).
I'm looking forward to your remarks and I will be happy to modify the code to fit it into the project.
Cheers
Andreas