MAINT: fix invalid escape seqs by tylerjereddy · Pull Request #2885 · MDAnalysis/mdanalysis

tylerjereddy · 2020-08-06T02:48:47Z

invalid escape sequences in str/bytes are deprecated
in newer versions of Python: https://bugs.python.org/issue27364
fix the various DeprecationWarning: invalid escape sequence \..
warnings emitted by our full test suite, mostly by using raw strings
r'...' to allow use of internal \ for strings targeted for regular expressions
and for docstrings that use \ for diagrams and other things
full test suite warning count drops slightly from 30,298 to 30,275 in my Python
3.8 venv

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

pep8speaks · 2020-08-06T02:48:52Z

Hello @tylerjereddy! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file package/MDAnalysis/analysis/data/filenames.py:

Line 79:29: E261 at least two spaces before inline comment
Line 79:80: E501 line too long (81 > 79 characters)

In the file package/MDAnalysis/analysis/encore/similarity.py:

Line 807:80: E501 line too long (81 > 79 characters)

In the file package/MDAnalysis/analysis/hole2/hole.py:

Line 203:80: E501 line too long (102 > 79 characters)
Line 416:80: E501 line too long (102 > 79 characters)
Line 785:80: E501 line too long (83 > 79 characters)

In the file package/MDAnalysis/core/groups.py:

Line 412:80: E501 line too long (80 > 79 characters)

In the file package/MDAnalysis/core/topologyattrs.py:

Line 977:80: E501 line too long (81 > 79 characters)

In the file testsuite/MDAnalysisTests/utils/test_meta.py:

Line 47:80: E501 line too long (87 > 79 characters)

Comment last updated at 2020-10-02 21:02:27 UTC

codecov · 2020-08-06T04:11:19Z

Codecov Report

Merging #2885 into develop will not change coverage.
The diff coverage is 81.81%.

@@           Coverage Diff            @@
##           develop    #2885   +/-   ##
========================================
  Coverage    93.05%   93.05%           
========================================
  Files          186      186           
  Lines        24609    24609           
  Branches      3187     3187           
========================================
  Hits         22900    22900           
  Misses        1661     1661           
  Partials        48       48

Impacted Files	Coverage Δ
package/MDAnalysis/analysis/dihedrals.py	`100.00% <ø> (ø)`
...is/analysis/encore/clustering/ClusterCollection.py	`70.58% <0.00%> (ø)`
package/MDAnalysis/analysis/encore/similarity.py	`91.56% <ø> (ø)`
package/MDAnalysis/analysis/encore/utils.py	`89.05% <ø> (ø)`
package/MDAnalysis/analysis/gnm.py	`96.80% <ø> (ø)`
package/MDAnalysis/coordinates/H5MD.py	`94.55% <ø> (ø)`
package/MDAnalysis/core/groups.py	`98.58% <ø> (ø)`
package/MDAnalysis/lib/distances.py	`98.41% <ø> (ø)`
package/MDAnalysis/lib/transformations.py	`85.31% <ø> (ø)`
...ckage/MDAnalysis/analysis/encore/confdistmatrix.py	`77.52% <50.00%> (ø)`
... and 9 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 be9380b...d83c782. Read the comment docs.

jbarnoud

Looks good to me.

jbarnoud · 2020-08-06T05:37:30Z

Maybe you could add an entry in the CHANGELOG?

tylerjereddy · 2020-08-07T02:44:44Z

hmm, I could, nothing user-facing though? Perhaps I could turn those warnings into errors, and then it would warrant an entry and prevent "regressions."

orbeckst · 2020-08-07T23:08:15Z

I like your idea of preventing regressions – please do!

tylerjereddy · 2020-08-09T23:50:25Z

Ok, see the second commit for the PR revisions based on feedback

orbeckst

thank you!

EDIT: seems to have triggered more errors...

orbeckst · 2020-08-10T22:32:58Z

Well... that's now a bottomless pit of fixing strings, and least when I try it locally... I started and will push updates but that might be a lot of work.

tylerjereddy · 2020-08-10T23:40:24Z

Hmm, maybe not worth too much time for now then. I wonder why I only caught cases in the test suite proper when I did it locally. Maybe the fresh rebuild in CI on top of the site.cfg change or something.

orbeckst · 2020-08-10T23:42:03Z

I have a fair amount done... but just ran into other minor things.

orbeckst · 2020-08-11T00:17:06Z

Now passes locally. Sorry, I force-pushed without too much thinking.

orbeckst · 2020-08-12T01:00:06Z

I think I'll want to merge a working develop into this one before merging, just to see that the CI really passes.

orbeckst · 2020-08-12T16:46:26Z

@jbarnoud can I please leave it to you to shepherd the PR to completion? Ping me if you need me to do anything else.

orbeckst · 2020-08-17T09:29:38Z

No idea why sphinx is suddenly complaining in https://travis-ci.com/github/MDAnalysis/mdanalysis/jobs/372482678

Warning, treated as error:

/home/travis/build/MDAnalysis/mdanalysis/package/doc/sphinx/source/documentation_pages/core_modules.rst:36:toctree contains reference to document 'documentation_pages/core/topologyattrs' that doesn't have a title: no link will be generated

sigh...

* invalid escape sequences in `str/bytes` are deprecated in newer versions of Python: https://bugs.python.org/issue27364 * fix the various `DeprecationWarning: invalid escape sequence \..` warnings emitted by our full test suite, mostly by using raw strings `r'...'` to allow use of internal `\` * full test suite warning count drops slightly from 30,298 to 30,276

* invalid escape sequence usage is now treated as an error in our full test suite to prevent regressions * CHANGELOG update

- mostly convert strings to raw strings - removed a number of embedded newlines from raw strings (because they do not work in raw strins as expected) - fixed heading of MDAnalysis/core/topologyattrs.py - fixed reST in Janin

orbeckst · 2020-10-02T21:03:52Z

Fixed the merge conflict, rebased against develop, consolidated commits (so that it can just be merged), and fixed the docs so that they build.

Locally

tests pass ================================ 17007 passed, 109 skipped, 4 xfailed, 2 xpassed, 38380 warnings in 713.78s (0:11:53) =================================
docs build

orbeckst · 2020-10-02T23:11:59Z

This is now looking good and the lower diff coverage comes from changing strings in parts of files that haven't been tested before.

tylerjereddy · 2020-10-10T22:36:18Z

Thanks, this turned out to be more work than I could handle on a short timescale.

tylerjereddy added the maintainability label Aug 6, 2020

tylerjereddy force-pushed the treddy_invalid_escape_fix branch from 5730c5a to 1ef58c6 Compare August 6, 2020 02:53

jbarnoud approved these changes Aug 6, 2020

View reviewed changes

orbeckst approved these changes Aug 10, 2020

View reviewed changes

orbeckst force-pushed the treddy_invalid_escape_fix branch from bd38a09 to 1b23d48 Compare August 11, 2020 00:16

orbeckst assigned jbarnoud Aug 12, 2020

tylerjereddy and others added 3 commits October 2, 2020 11:49

MAINT: PR 2885 revisions

3337942

* invalid escape sequence usage is now treated as an error in our full test suite to prevent regressions * CHANGELOG update

MAINT: cleaned up escape strings and fixed reST

d83c782

- mostly convert strings to raw strings - removed a number of embedded newlines from raw strings (because they do not work in raw strins as expected) - fixed heading of MDAnalysis/core/topologyattrs.py - fixed reST in Janin

orbeckst force-pushed the treddy_invalid_escape_fix branch from 330b38d to d83c782 Compare October 2, 2020 21:02

orbeckst merged commit 542daa9 into MDAnalysis:develop Oct 2, 2020

tylerjereddy deleted the treddy_invalid_escape_fix branch October 10, 2020 22:35

Conversation

tylerjereddy commented Aug 6, 2020 • edited by orbeckst Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

pep8speaks commented Aug 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2020-10-02 21:02:27 UTC

Uh oh!

codecov bot commented Aug 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jbarnoud left a comment

Choose a reason for hiding this comment

Uh oh!

jbarnoud commented Aug 6, 2020

Uh oh!

tylerjereddy commented Aug 7, 2020

Uh oh!

orbeckst commented Aug 7, 2020

Uh oh!

tylerjereddy commented Aug 9, 2020

Uh oh!

orbeckst left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orbeckst commented Aug 10, 2020

Uh oh!

tylerjereddy commented Aug 10, 2020

Uh oh!

orbeckst commented Aug 10, 2020

Uh oh!

orbeckst commented Aug 11, 2020

Uh oh!

orbeckst commented Aug 12, 2020

Uh oh!

orbeckst commented Aug 12, 2020

Uh oh!

orbeckst commented Aug 17, 2020

Uh oh!

orbeckst commented Oct 2, 2020

Uh oh!

orbeckst commented Oct 2, 2020

Uh oh!

tylerjereddy commented Oct 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tylerjereddy commented Aug 6, 2020 •

edited by orbeckst

Loading

pep8speaks commented Aug 6, 2020 •

edited

Loading

codecov bot commented Aug 6, 2020 •

edited

Loading

orbeckst left a comment •

edited

Loading