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

FIX: Mark strings containing regex escapes as raw #3106

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

effigies
Copy link
Member

Summary

Ran into some deprecation errors working on #3099. The escape characters were easy to handle.

Acknowledgment

  • (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

@@ -474,15 +474,16 @@ def _make_matlab_command(self, _):
script += "condnames=names;\n"
else:
if self.inputs.use_derivs:
script += "pat = 'Sn\([0-9]*\) (.*)';\n"
script += r"pat = 'Sn\([0-9]*\) (.*)';" "\n"
Copy link
Member Author

Choose a reason for hiding this comment

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

Hopefully we test these. I'm less comfortable with these, since they need to work when written to MATLAB scripts.

Copy link
Member

Choose a reason for hiding this comment

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

as long as we are still running the spm workflows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Codecov doesn't seem to hit them.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've tested:

>>> a = ""
>>> a += r"\s+" "\nnextline"                                                                   
>>> print(a)                                                                                   
\s+
nextline

So rawstring/string concatenation works fine, and the += is lower precedence, so I'm reasonably confident in this change.

Merging.

@codecov
Copy link

codecov bot commented Nov 25, 2019

Codecov Report

Merging #3106 into master will increase coverage by 0.59%.
The diff coverage is 10.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3106      +/-   ##
==========================================
+ Coverage   67.23%   67.83%   +0.59%     
==========================================
  Files         294      295       +1     
  Lines       39245    39263      +18     
  Branches     5162     5172      +10     
==========================================
+ Hits        26386    26633     +247     
+ Misses      12185    11924     -261     
- Partials      674      706      +32
Flag Coverage Δ
#smoketests 51.23% <16.66%> (ø) ⬆️
#unittests 65.06% <10.52%> (+0.84%) ⬆️
Impacted Files Coverage Δ
nipype/interfaces/spm/model.py 42.27% <0%> (ø) ⬆️
nipype/interfaces/diffusion_toolkit/odf.py 68.54% <0%> (ø) ⬆️
nipype/sphinxext/plot_workflow.py 13.79% <0%> (ø) ⬆️
nipype/interfaces/freesurfer/utils.py 62.88% <0%> (+2.13%) ⬆️
nipype/interfaces/dipy/base.py 77.87% <100%> (ø) ⬆️
nipype/interfaces/elastix/utils.py 28.39% <12.5%> (ø) ⬆️
nipype/pipeline/plugins/legacymultiproc.py 62.81% <0%> (-0.51%) ⬇️
nipype/testing/__init__.py 88.88% <0%> (ø)
nipype/interfaces/spm/preprocess.py 56.93% <0%> (+0.32%) ⬆️
nipype/utils/filemanip.py 77.59% <0%> (+0.47%) ⬆️
... 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 0225a45...d7cf744. Read the comment docs.

@@ -189,7 +189,7 @@ def dipy_to_nipype_interface(cls_name, dipy_flow, BaseClass=DipyBaseInterface):
parser = IntrospectiveArgumentParser()
flow = dipy_flow()
parser.add_workflow(flow)
default_values = inspect.getargspec(flow.run).defaults
default_values = inspect.getfullargspec(flow.run).defaults
Copy link
Member Author

Choose a reason for hiding this comment

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

Guess I fixed another deprecation warning. This works for all Py3.

@effigies
Copy link
Member Author

Anybody up for a review?

@satra
Copy link
Member

satra commented Dec 12, 2019

in general looks fine to me.

@effigies effigies merged commit 66462d3 into nipy:master Dec 12, 2019
@effigies effigies deleted the mnt/deprecations branch December 13, 2019 03:15
@effigies effigies added this to the 1.4.0 milestone Jan 6, 2020
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.

2 participants