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

DOC: Specify encoding of rst file, remove an ambiguity in an SVD example #8706

Merged
merged 3 commits into from
Apr 12, 2018

Conversation

pvanmulbregt
Copy link
Contributor

During execution of python runtests.py -g --regime-check open a text file using encoding utf-8 under Python 3, leave as is for Py2.
Multiply a 2x1 matrix by the sign of the first element to remove some ambiguity in a linalg.svd null_space example.

Closes gh-8703.

@pvanmulbregt pvanmulbregt added the Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org label Apr 10, 2018
if sys.version_info.major <= 2:
text = open(fname).read()
else:
text = open(fname, encoding='utf-8').read()
Copy link
Member

Choose a reason for hiding this comment

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

This could probably be:

with codecs.open(fname, encoding='utf-8') as fid:
    text = fid.read()

That should work on 2 and 3, and properly close the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would that not change the semantics slightly? I believe that codecs.open() opens in binary mode and doesn't do '\n' conversion, and in Py2 it would always return Unicode. I did think about using io.open(fname, encoding='utf-8') but didn't because text would now be a Unicode object in Py2 and downstream code may not be expecting Unicode.

Copy link
Member

Choose a reason for hiding this comment

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

Well if it broke things we should probably fix those downstream things to handle unicode. But for now (esp. since Py2 support will be dropped within a year) we can go with your solution, but please change it to use the context manager to properly close the file.

@ev-br ev-br merged commit b21e246 into scipy:master Apr 12, 2018
@ev-br
Copy link
Member

ev-br commented Apr 12, 2018

Looks good. Thanks @pvanmulbregt

@ev-br ev-br added this to the 1.1.0 milestone Apr 12, 2018
@pvanmulbregt pvanmulbregt deleted the refguide-check branch April 13, 2018 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants