-
Notifications
You must be signed in to change notification settings - Fork 29
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
Keep string data as python str #536
Conversation
Would it make sense to add a test for this fix? |
@JuliaSprenger thanks. Added a test, if you're happy, I guess you may approve and merge ;) |
I don't have the rights to merge. @mpsonntag can you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Thanks! Are you planning a minor release with the fix soon? In that case I could adjust the neo dependency already now. |
actually yes. As soon as the other PRs pass and are merged. I need the changes in PR #537 for some current stuff. That pr raises an interesting question: I think it fair to raise OutOfBound exceptions in our |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase LGTM
Hi again! It seems all other PRs are merged. So is there a version 1.5.3. release coming soon? |
@JuliaSprenger Done! :) |
Thank you :) |
With b3c21c3 we fixed the reading of 2D string data. Unfortunately, this implicitly converted python strings to np.str_. With this pr, we keep them as python strings.
fixes #534