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

Coerce expression #1137

Merged
merged 18 commits into from Jun 25, 2015

Conversation

Projects
None yet
2 participants
@cpcloud
Member

cpcloud commented Jun 16, 2015

closes #1050

assert str(s.coerce('int64')) == "s.coerce(to='int64')"
@pytest.mark.xfail(raises=AttributeError, reason='Should this be valid?')

This comment has been minimized.

@llllllllll

llllllllll Jun 16, 2015

Member

Is the question here if we should be able to cast down from float64 to float32? I think the idea of casting records like this is pretty cool.

This comment has been minimized.

@cpcloud

cpcloud Jun 16, 2015

Member

The question is: Is casting an entire set of rows on a subset of columns (possibly all the columns as well) useful? How often is it the case that you want to change the type of a set of columns? I honestly don't know. I know that I do this in one particular case when I'm shoving a DataFrame that contains strings into a bcolz ctable in Python3.4.

This comment has been minimized.

@llllllllll

llllllllll Jun 16, 2015

Member

This would be nice because there are some places that I have tz-aware datetimes in dataframes so the type is object and I would like to coerce all of the columns to datetime instead of object.

This comment has been minimized.

@cpcloud

cpcloud Jun 20, 2015

Member

In the case of tz-aware datetimes they are cast to object because pandas doesn't have a way to manage a contiguous array of tz-aware datetimes (yet).

This comment has been minimized.

@cpcloud

cpcloud Jun 20, 2015

Member

Oh actually, I see what you mean ... they might be inferred as object, but you want whatever you're sending them to to see that they are datetimes

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 16, 2015

Can you add a series coerce?

@cpcloud

This comment has been minimized.

Member

cpcloud commented Jun 16, 2015

yep, just haven't gotten to it yet

@@ -705,6 +705,21 @@ def dshape(self):
return dshape(self._dshape)
class Coerce(Expr):

This comment has been minimized.

@llllllllll

llllllllll Jun 19, 2015

Member

can we add a docstring here with some examples?

This comment has been minimized.

@cpcloud

cpcloud Jun 19, 2015

Member

Yep will do

def test_coerce():
s = symbol('s', var * float32)
assert str(s.coerce('int64')) == "s.coerce(to='int64')"

This comment has been minimized.

@llllllllll

llllllllll Jun 19, 2015

Member

Can we check the dshape of the coerce node to make sure that it worked properly?

This comment has been minimized.

@cpcloud

cpcloud Jun 19, 2015

Member

Good point

@@ -22,7 +22,7 @@
__all__ = ['Expr', 'ElemWise', 'Field', 'Symbol', 'discover', 'Projection',
'projection', 'Selection', 'selection', 'Label', 'label', 'Map',
'ReLabel', 'relabel', 'Apply', 'Slice', 'shape', 'ndim', 'label',
'symbol']
'symbol', 'Coerce', 'coerce']

This comment has been minimized.

@llllllllll

llllllllll Jun 21, 2015

Member

Should the coerce function be available as a standalone function from the top level package or do you think this is most useful as a method

This comment has been minimized.

@cpcloud

cpcloud Jun 22, 2015

Member

I think I'll just leave it as a method for now and remove it from __all__

@llllllllll

This comment has been minimized.

Member

llllllllll commented Jun 21, 2015

Just had one comment on the visibility of the coerce function; but, this looks good to me

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

@cpcloud cpcloud merged commit 27fadde into blaze:master Jun 25, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@cpcloud cpcloud deleted the cpcloud:coerce branch Jun 25, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment