Skip to content

Add micro- and millsecond period intervals. #2145

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

Closed
cpcloud opened this issue Oct 29, 2012 · 12 comments
Closed

Add micro- and millsecond period intervals. #2145

cpcloud opened this issue Oct 29, 2012 · 12 comments
Milestone

Comments

@cpcloud
Copy link
Member

cpcloud commented Oct 29, 2012

I'm totally willing to implement this myself, but I'm not exactly sure where to begin. I tried adding a microsecond period, but it seems like there are many low level changes required (plib.pyx, period.py (obviously), period.c, period.h, datetime.pyx, datetime.pxd and others that I can't remember off the top of my head).

Is there some documentation or a list of files and functions that need to be changed to add a period interval? If not, I can try and assemble one.

@wesm
Copy link
Member

wesm commented Oct 29, 2012

The work to do is mainly in period.c. period.h has some constants and functions prototypes. plib.pyx is a simple cython wrapper for period.c/h. There's no (I'm pretty sure?) interaction with datetime.pyx/pxd

@cpcloud
Copy link
Member Author

cpcloud commented Oct 30, 2012

One important detail that I've run into is that the abstime member of the date_info is in the smallest unit used. Makes sense, just wanted to document it here. The implications of this are largely where most of the work lies.

@cpcloud
Copy link
Member Author

cpcloud commented Nov 3, 2012

In period.h and plib.pyx there is the function signature getAbsTime. However, it is not called in period.c nor is it used in plib.pyx. period.c has calls to a function called get_abs_time whose signature is not in period.h. Is it okay to make get_abs_time a static function?

@wesm
Copy link
Member

wesm commented Nov 4, 2012

I'm really not a C expert, so you can make any refactorings you like as long as the test suite passes. You'll have to use 64-bit integers for micro- and nanoseconds since a whole day's worth won't fit in a 32-bit integer. If you look around for stdint.h includes you will find a portable way to get int64_t in there if it's not included already

@cpcloud cpcloud closed this as completed Dec 20, 2012
@cpcloud cpcloud reopened this Dec 20, 2012
@cpcloud
Copy link
Member Author

cpcloud commented Dec 24, 2012

I want to submit a pull request for this issue. I've just (re)-read the pandas contributing documentation and I'm not sure I understand whether I need to rebase. I have an upstream branch that I'm constantly merging with my fork of pandas git master and that I'm also merging into the feature branch for this issue (I check for upstream changes at least once per day). In that case it would seem that I do not need to worry about rebasing, but I just want to make sure.

@ghost
Copy link

ghost commented Dec 24, 2012

Rebasing will ensure that your commits apply cleanly on top of upstream (at that moment at least),
but it's better to leave the conflicts as they are then to merge master back into your own branch, you
never want to do that if you'll submit the branch as a PR.

If your git-fu can handle it, then rebase on top of upstream and force push to replace the old commits
with the new history, and if you need to make changes/fixes, you can use interactive rebase to squash and
reorder commits in your PR to make the history as clean as possible.

@cpcloud
Copy link
Member Author

cpcloud commented Dec 24, 2012

So for future requests I should never merge pandas git master back into my feature branch and just work on the branch from whatever state master was in when I created the branch?

@jreback
Copy link
Contributor

jreback commented Dec 24, 2012

here's the way I have setup

master branch where I don't do work, and
feature_branch, which I get by git co -b feature_branch

you may need to do

git br feature_branch --set-upstream master (if git pull does not work when in feature_branch)

to update master:
git co master git pull

in theory your feature branch is now updated
but if not

git co feature_branch git pull --rebase
will reapply all of your commits and essentially rewrite the commit history in your feature branch

you may also need to do git push myfork feature_branch --force to push

as long as someone else is not working ON THE SAME things in master, this will work to update your feature branch to master and not pollute the history

@ghost
Copy link

ghost commented Dec 24, 2012

@cpcloud, yes. Rebase if you'd like to be neater, but never merge upstream into a branch
you intend to PR.

You can think of rebasing as cherry picking all your commits one by one on top a new
starting point, if you're familiar with that.

jreback's recipe is just fine.

@cpcloud
Copy link
Member Author

cpcloud commented Jan 8, 2013

I went through almost every commit and rebased and squashed as many as I could, but if I look at what this looks like if I were to submit a pull request I still get a bunch of "Merge with upstream/master ..." commits. Is it okay to submit this, or is there a way to get only the commits that don't have that line in the title? I promise I'll never merge my branch with upstream again in any future pull requests. :|

@ghost
Copy link

ghost commented Jan 8, 2013

By all means, if have something to contribute to the porject but got tangled up with git , open
up a PR and it'll get sorted. You can get it right in your next PR.

If you used "rebase --interactive" you can just "drop" the merge commits,
there's no reason why they should remain in your PR.
Or you can can just create a new branch of current git master, and "git cherry-pick"
the commits in your topic branch one by one, excluding the merge commits.

Both of these are similar in terms of the end result.

If you're an emacs user, magit has wonderful support for interactive rebasing, I use it constantly.

@ghost
Copy link

ghost commented Mar 14, 2013

Continued in #2670

@ghost ghost closed this as completed Mar 14, 2013
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants