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

Avoid deprecated 'U' mode when opening files #2187

Merged
merged 1 commit into from
Jul 2, 2018
Merged

Avoid deprecated 'U' mode when opening files #2187

merged 1 commit into from
Jul 2, 2018

Conversation

jdufresne
Copy link
Contributor

@jdufresne jdufresne commented Nov 1, 2016

Instead, use io.open() wrapper to handle universal newlines for all Python versions. (Different approach taken, see below.)

Fixes warning "DeprecationWarning: 'U' mode is deprecated" in tests.

@wiredfool
Copy link
Member

IIRC, there was some pain around trying this previously.

@jdufresne
Copy link
Contributor Author

Do you have any information on the previous attempt? If an edge case does exist, it doesn't look to be captured in a test.

@wiredfool
Copy link
Member

I think it's from the pr around 9b35a45

Looking around that era, it does look like the U was just carried forward at that time, rather than being added then.

@jdufresne
Copy link
Contributor Author

Looking at the EPS spec, it looks like Python's "universal new lines" will always be incorrect. From the spec:

2.9 Miscellaneous Constraints

...

Lines must be terminated with one of the following combinations of characters: CR, LF, CR LF, or LF CR.

CR is the carriage return character and LF is the line feed character (decimal ASCII 13 and 10, respectively).

Python's universal newlines does not handle the '\n\r' case.

Therefore, I think the PSFile wrapper should be used in all cases.

I have updated the PR with this chage as well as updated the tests to now test against all combinations of line endings.

@jdufresne
Copy link
Contributor Author

jdufresne commented Nov 7, 2016

As a nice side effect, this change also fixes the orphaned open file that was never explicitly closed. Thus leading to fewer ResourceWarnings during tests with warnings enabled.

@jdufresne
Copy link
Contributor Author

Rebased to latest master.

@hugovk
Copy link
Member

hugovk commented Jul 1, 2018

I suggest we merge this after this quarterly release so it has some time to sit in master.

@codecov
Copy link

codecov bot commented Jul 1, 2018

Codecov Report

Merging #2187 into master will decrease coverage by 0.75%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2187      +/-   ##
==========================================
- Coverage   84.03%   83.28%   -0.76%     
==========================================
  Files         170      168       -2     
  Lines       23776    22616    -1160     
  Branches     2825     2795      -30     
==========================================
- Hits        19981    18836    -1145     
+ Misses       3795     3780      -15
Impacted Files Coverage Δ
src/PIL/EpsImagePlugin.py 89.13% <100%> (-0.28%) ⬇️
src/PIL/GdImageFile.py 0% <0%> (-96.16%) ⬇️
src/PIL/WalImageFile.py 0% <0%> (-92.86%) ⬇️
src/PIL/ImageDraw2.py 0% <0%> (-84.38%) ⬇️
src/PIL/ImagePath.py 60% <0%> (-40%) ⬇️
src/libImaging/File.c 76% <0%> (-8.62%) ⬇️
src/_imagingmorph.c 85.4% <0%> (-6.57%) ⬇️
src/PIL/ImageDraw.py 88.05% <0%> (-4.87%) ⬇️
src/libImaging/Geometry.c 71.75% <0%> (-2.55%) ⬇️
src/_imagingtk.c 85.71% <0%> (-2.53%) ⬇️
... and 57 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 885c9cb...4ea7902. Read the comment docs.

Instead, use PSFile() wrapper to handle all newline in the EPS spec.

Update line ending tests to handle all combinations of '\n' and '\r'.

Fixes warning "DeprecationWarning: 'U' mode is deprecated" in tests.
@jdufresne
Copy link
Contributor Author

Rebased to resolve merge conflicts.

@hugovk hugovk merged commit 4d88e28 into python-pillow:master Jul 2, 2018
@jdufresne jdufresne deleted the open-warning branch March 17, 2019 16:58
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

4 participants