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.

Sign up for GitHub

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

Jump to bottom

Replace re with regex #457

Merged
merged 4 commits into from
Aug 3, 2020
Merged

Replace re with regex #457

merged 4 commits into from
Aug 3, 2020

Conversation

dreamerlin
Copy link
Contributor

This PR takes regex to replace re to solve the issue: "re.error: bad escape \U at position 2", which usually happens in Windows platform.

@codecov
Copy link

codecov bot commented Aug 2, 2020 •
edited
Loading

Codecov Report

Merging #457 into master will increase coverage by 0.78%.
The diff coverage is 78.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #457      +/-   ##
==========================================
+ Coverage   62.42%   63.20%   +0.78%     
==========================================
  Files         126      127       +1     
  Lines        7116     7170      +54     
  Branches     1255     1265      +10     
==========================================
+ Hits         4442     4532      +90     
+ Misses       2469     2413      -56     
- Partials      205      225      +20
Flag Coverage Δ
#unittests 63.20% <78.33%> (+0.78%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmcv/ops/nms.py 13.27% <0.00%> (-0.89%) ⬇️
mmcv/runner/__init__.py 100.00% <ø> (ø)
mmcv/runner/base_runner.py 69.60% <25.00%> (+2.77%) ⬆️
mmcv/utils/config.py 88.27% <33.33%> (-0.62%) ⬇️
mmcv/cnn/bricks/plugin.py 93.75% <50.00%> (-6.25%) ⬇️
mmcv/runner/hooks/sync_buffer.py 64.28% <66.66%> (+64.28%) ⬆️
mmcv/runner/hooks/ema.py 90.69% <90.69%> (ø)
mmcv/runner/hooks/__init__.py 100.00% <100.00%> (ø)
... and 4 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 1830347...e77f24f. Read the comment docs.

hellock
hellock reviewed Aug 2, 2020
View reviewed changes
mmcv/cnn/bricks/plugin.py Outdated
@@ -1,5 +1,6 @@
import inspect
import re

import regex as re
Copy link
Member

Choose a reason for hiding this comment

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

Use regex only on windows.

hellock
hellock reviewed Aug 2, 2020
View reviewed changes
requirements.txt Outdated
@@ -1,4 +1,5 @@
addict
numpy
pyyaml
regex
Copy link
Member

Choose a reason for hiding this comment

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

regex is not compulsory.

Copy link
Member

Choose a reason for hiding this comment

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

regex;sys_platform=='win32'

@hellock hellock merged commit 9de04d2 into open-mmlab:master Aug 3, 2020
wxzs5 pushed a commit to wxzs5/mmcv that referenced this pull request Aug 6, 2020
@dreamerlin
Replace re with regex (open-mmlab#457)
95620c4 
* replace `re` with `regex`

* add regex in requirements

* use regex on windows and re on linux

* update requirements.txt
@dreamerlin dreamerlin deleted the regex branch September 2, 2020 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hellock hellock hellock approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dreamerlin @hellock