Skip to content
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

Add encoder to handle masked numbers. #159

Merged
merged 13 commits into from
Dec 12, 2014
Merged

Conversation

theengineear
Copy link
Contributor

@chriddyp , it'd be cool to get your input on this one, I think you wrote the encoders in here originally. I need to write a test before we pass this off still too.

@chriddyp
Copy link
Member

Yeah, great! I think instead of checking for math.isnan we should be doing something like pd.isnull and making a copy of the series and then pd.fillna'ing with None
Off the top of my head, there is also pandas.core.index.Index which needs to be type checked and converted to list. pd.fillna should catch np.NaN and pd.NaT, but it's worth checking!

@theengineear
Copy link
Contributor Author

@chriddyp do we have tests to ensure that data isn't mutated when it goes through the encoder? Just making copies of all the things we're encoding and testing that they're the same after the tests?

@@ -72,79 +73,115 @@ def ensure_dir_exists(directory):


### Custom JSON encoders ###
class NotEncodable(Exception):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's subclass a PlotlyError here?

errr... maybe it doesn't matter cause we're catching it anyways... nbd

return None

if isinstance(obj, (datetime.datetime, datetime.date)):
if obj.microsecond != 0:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you replace this with if obj.microsecond?

o.strftime('%Y-%m-%d')
for o in obj]
else:
raise NotEncodable
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's probably a more duck-typey way to do this. but it's not worth the time i think. i.e., just expecting attributes to exist etc...

@theengineear
Copy link
Contributor Author

@chriddyp is the 👨

@theengineear
Copy link
Contributor Author

🎉 nice!

chriddyp added a commit that referenced this pull request Dec 12, 2014
Add encoder to handle masked numbers.
@chriddyp chriddyp merged commit d309589 into master Dec 12, 2014
@chriddyp chriddyp deleted the jsonify-masked-numbers branch December 12, 2014 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants