-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
datasets.get_rdataset string decode error on python 3 #1055
Comments
Yeah, the error is intended because I didn't know how to properly do the decoding on Python 3 and wasn't necessarily expecting unicode from an R dataset. |
We might be able to postpone this until the py2 py3 common code base conversion, where we have to decide and figure out what to do with strings. |
What are the expectations for if something is cached using 2.x that we're able to read it from 3.x? That's the only hold up for me right now. I'd rather not support it, but it should be doable and we probably should support this. |
I could also mangle the cached name to include the python version, so the Python 3 will never find a Python 2 cached item. |
This is the implicit expectation now actually. It was just never actually checked for. |
Nevermind, it's an easy fix. |
Is this related to caching, I'm using a new python 3.3 version with installed statsmodels. Do we share cache directories across python version and different statsmodels folders? But in either case, we are just caching the csv file. Isn't that the same across python versions? |
No, it's not related to caching but fixing it brought up a caching bug for me since I had this file saved from 2.7. Basically just a bytes vs. str issue. |
so nothing I should do to help on RDatasets side? |
No you're fine. Just lazy coding catching up to me. |
missing cross-link to PR #1057 |
trying to run Skipper's example from issue #805 on python 3.3
fails with a UnicodeDecodeError
I guess we don't have a test for this.
I ran several examples on python 3 before the release, but none that used get_rdataset
The text was updated successfully, but these errors were encountered: