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

ENH: Add test for encoding #9615

Merged
merged 3 commits into from
Dec 21, 2018
Merged

ENH: Add test for encoding #9615

merged 3 commits into from
Dec 21, 2018

Conversation

larsoner
Copy link
Member

Closes #9606.

First commit should flag as red on the fast Travis build. Once it does, I'll correct the errors it's pointing out.

@larsoner
Copy link
Member Author

On the first commit I killed all CIs except for Azure (no account there for me yet) and the one relevant Travis run which yielded:

$ grep -rlIP '[^\x00-\x7F]' scipy | grep '\.pyx\?' | sort > unicode.out; grep -rlI '# -\*- coding: \(utf-8\|latin-1\) -\*-' scipy | grep '\.pyx\?' | sort > coding.out; comm -23 unicode.out coding.out > test_code.out; cat test_code.out;  test \! -s test_code.out
scipy/io/__init__.py
scipy/stats/_continuous_distns.py
scipy/stats/_stats_mstats_common.py
The command "grep -rlIP '[^-�]' scipy | grep '\.pyx\?' | sort > unicode.out; grep -rlI '# -\*- coding: \(utf-8\|latin-1\) -\*-' scipy | grep '\.pyx\?' | sort > coding.out; comm -23 unicode.out coding.out > test_code.out; cat test_code.out;  test \! -s test_code.out
" exited with 1.

Now re-running with all CIs, should be ready for review/merge as they should come back green since the only job that should be affected is green.

@larsoner larsoner added defect A clear bug or issue that prevents SciPy from being installed or used as expected backport-candidate This fix should be ported by a maintainer to previous SciPy versions. labels Dec 19, 2018
@larsoner
Copy link
Member Author

I added a commit to remove the en-dashes in functions where we had r""" docstrings, and added one or two u prefixes to docstrings that used unicode. This should make it backport-able to the 1.2 / Python2 LTS release.

@rgommers
Copy link
Member

Works for me, thanks @larsoner.

@tylerjereddy seems okay to me as backport, will leave that to you to decide though.

tylerjereddy added a commit to tylerjereddy/scipy that referenced this pull request Dec 28, 2018
rgommers added a commit that referenced this pull request Jan 3, 2019
MAINT: prepare branch for 1.2.1 & backport gh-9615
@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Feb 1, 2019
@WarrenWeckesser
Copy link
Member

Given that we no longer support Python 2, is the check for unicode characters still necessary? I'm working on a pull request in which a test docstring refers to π (the actual value of π) and np.pi (the floating point approximation of π). Using the symbol π makes the comments easier to read.

@rgommers
Copy link
Member

rgommers commented Jun 4, 2019

Given that we no longer support Python 2, is the check for unicode characters still necessary?

It's not, we can remove it. It took a while to find: PEP 3120 specifies that UTF-8 is the default encoding for Python 3.x, and so we can use UTF-8 characters without having to declare an encoding or cause any issues.

@WarrenWeckesser if you do this, could you do some sanity checking that there's no surprises (e.g. ipython and python interpreters show docstring fine)?

@WarrenWeckesser
Copy link
Member

Last year (two comments up from here) I asked about removing this check. @rgommers confirmed that we can remove it, but suggested looking into how programs such as ipython handle Unicode characters in docstring. This never made it to the top of my to-do list, but since then there has been some work in NumPy that looks relevant.

Over in numpy/numpy#15666, @rossbar added the use of Unicode characters in the string representation of NumPy's polynomial. It turns out that using Unicode in a Windows terminal can cause problems; see the discussion starting at numpy/numpy#15666 (comment). The problem is that the font used by the terminal might not have full Unicode support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected
Projects
None yet
4 participants