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

ENH: Adds datetime arithmetic with timedeltas #1112

Merged
merged 9 commits into from Jun 9, 2015

Conversation

Projects
None yet
3 participants
@llllllllll
Member

llllllllll commented Jun 4, 2015

I wanted to be able to add timedeltas to interactive symbols so I made it so.

In [5]: ds.quandl.vix.knowledge_date
Out[5]: 
   knowledge_date
0      1990-01-02
1      1990-01-03
2      1990-01-04
3      1990-01-05
4      1990-01-08
5      1990-01-09
6      1990-01-10
7      1990-01-11
8      1990-01-12
9      1990-01-15
...

In [6]: ds.quandl.vix.knowledge_date + datetime.timedelta(days=1)
Out[6]: 
   knowledge_date
0      1990-01-03
1      1990-01-04
2      1990-01-05
3      1990-01-06
4      1990-01-09
5      1990-01-10
6      1990-01-11
7      1990-01-12
8      1990-01-13
9      1990-01-16
...

datashape already supported this I just needed to add a _(r)?add and _(r)?sub to isdatelike

@mrocklin

This comment has been minimized.

Member

mrocklin commented Jun 4, 2015

Whoa. Cool.

@cpcloud cpcloud added this to the 0.8.1 milestone Jun 4, 2015

@cpcloud cpcloud added the enhancement label Jun 4, 2015

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

noice

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

ah looks like a dask failure

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

@llllllllll doesn't seem related to your change

@llllllllll llllllllll closed this Jun 4, 2015

@llllllllll llllllllll reopened this Jun 4, 2015

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

@llllllllll what backend are you using there?

@cpcloud cpcloud self-assigned this Jun 4, 2015

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 4, 2015

blaze client serving a dictionary of sqlalchemy metadata objects.

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

@llllllllll pull down master and push again, i just fixed the dask failure. it should take care of the failures we're seeing on travis here

@llllllllll llllllllll force-pushed the quantopian:date-arith branch from 23c3f0a to a8cb87d Jun 4, 2015

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

Can you add a test for this? You can write it against postgres if you want. I have an instance running locally that I can test against and Travis-CI does as well.

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 4, 2015

I could do pg and pandas, I imagine this will be common on dataframes, series, and datetimeindidcies.

What is the connection string to use for pg?

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

'postgresql://postgres@localhost/test::table'

table can be whatever you want

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

gimme a sec and i'll add a pg db to travis

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 4, 2015

I imagine I will need to manage the creation of the table and data inside the test?

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 4, 2015

yeah, but pytest fixtures make that easy enough. if you have any questions about that i'm happy to help. odo also has an example here: https://github.com/ContinuumIO/odo/blob/master/odo/backends/tests/test_postgres.py#L35-L48

@llllllllll llllllllll force-pushed the quantopian:date-arith branch from a8cb87d to daad744 Jun 4, 2015

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 4, 2015

@cpcloud I rebased against #1116 so your commits are pulled in here (under your name)

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 5, 2015

Excellent. I'll close the other PR then

@cpcloud cpcloud referenced this pull request Jun 5, 2015

Closed

Some travis things #1116

def test_timedelta_arith(sql_with_dts):
delta = timedelta(days=1)
dates = pd.Series(pd.date_range('2014-01-01', '2014-02-01'))
sym = symbol('s', '32 * datetime')

This comment has been minimized.

@cpcloud

cpcloud Jun 5, 2015

Member

you should be able to do symbol('s', discover(dates)) instead of explicitly writing down the dshape

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 5, 2015

@llllllllll tiny comment, which isn't critical to address, was just a thought. i hand tested this on redshift and works nicely. merging at some point today.

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 5, 2015

postgres looks like it has a lot of operations available for date arithmetic http://www.postgresql.org/docs/9.4/static/functions-datetime.html

might be nice to eventually implement some of these :)

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 5, 2015

I also added a test for numpy; however it only supports the np.timedelta64 objects instead of datetime.timedelta

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 5, 2015

PR on the way to skip numba optimizing when given a datetime / timedelta expression

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 5, 2015

Why, does numba not know how to handle these operations?

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 5, 2015

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 5, 2015

Is there a reason that the pr is against the quantopian fork, not the upstream?

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 5, 2015

yeah, because i wanted to use your test

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 5, 2015

kk

@llllllllll llllllllll force-pushed the quantopian:date-arith branch from 488f6c2 to f2c70c4 Jun 9, 2015

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 9, 2015

Rebased and fixed merge conflict

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 9, 2015

Looks like numba failed in the numpy test.

@llllllllll llllllllll closed this Jun 9, 2015

@llllllllll llllllllll reopened this Jun 9, 2015

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 9, 2015

it looks like you might've rebased away my commits that fixed that issue

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 9, 2015

Were they on this branch or on master? I now remember that you had a fix for this. Maybe you can re-pr your branch against this one?

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 9, 2015

i submitted a PR to your fork

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 9, 2015

let me see if i have the branch lying around

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 9, 2015

it'd be in your reflog if you haven't run git gc since the rebase as well

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 9, 2015

I just cherry-picked them over

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 9, 2015

cool

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 9, 2015

sorry about that

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 9, 2015

no problem!

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 9, 2015

@llllllllll ok to merge on passing?

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 9, 2015

yup

cpcloud added a commit that referenced this pull request Jun 9, 2015

Merge pull request #1112 from quantopian/date-arith
ENH: Adds datetime arithmetic with timedeltas

@cpcloud cpcloud merged commit 49f3ecd into blaze:master Jun 9, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment