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

BF::Fix inspect.getargspec deprecation warning in Python 3 #1413

Merged
merged 1 commit into from Feb 10, 2018

Conversation

naveenkumarmarri
Copy link
Contributor

for issue #1327

@codecov-io
Copy link

codecov-io commented Feb 3, 2018

Codecov Report

Merging #1413 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1413      +/-   ##
==========================================
+ Coverage   86.81%   86.82%   +0.01%     
==========================================
  Files         243      243              
  Lines       30215    30210       -5     
  Branches     3250     3250              
==========================================
- Hits        26231    26230       -1     
+ Misses       3244     3241       -3     
+ Partials      740      739       -1
Impacted Files Coverage Δ
dipy/workflows/multi_io.py 68.8% <100%> (-1.21%) ⬇️
dipy/reconst/forecast.py 92.22% <0%> (+2.07%) ⬆️

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 ec9b584...0e3764a. Read the comment docs.

@jhlegarreta
Copy link
Contributor

Awesome @naveenkumarmarri ! Thanks.

Fixes #1327.

Copy link
Member

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @naveenkumarmarri !

Even if I requested one change, it looks good.
Thanks !

spargs.remove('self')
defaults = specs.defaults
if sys.version_info[0] >= 3:
sig_object = inspect.signature(fnc)
Copy link
Member

Choose a reason for hiding this comment

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

why you do not use get_args_default in dipy/workflows/base.py

I think it will avoid to duplicate the code

@naveenkumarmarri
Copy link
Contributor Author

naveenkumarmarri commented Feb 6, 2018

@skoudoro made changes as per your feedback

@@ -1,11 +1,12 @@
import inspect
import numpy as np
import os
import sys
Copy link
Member

Choose a reason for hiding this comment

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

Unused import sys. Can you remove it ?

@skoudoro
Copy link
Member

skoudoro commented Feb 7, 2018

Thanks @naveenkumarmarri, any other requests @jhlegarreta?

If not, I merge it tomorrow

@jhlegarreta
Copy link
Contributor

Not on my side. Thanks for asking. Warnings have disappeared, so this is ready for merge. Thanks both @naveenkumarmarri and @skoudoro !

If the issue is not closed automatically (I think the Fixes PR# sentence has to be in the commit message and not the comments for this), I'll close it, and the corresponding card in the 0.14.0 project should then be moved to Done. Moving forward.

@skoudoro
Copy link
Member

skoudoro commented Feb 8, 2018

@naveenkumarmarri, can you rebase please?

Thank you

@skoudoro
Copy link
Member

Thank you @naveenkumarmarri ! merging !

@skoudoro skoudoro merged commit 6571f36 into dipy:master Feb 10, 2018
ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018
BF::Fix inspect.getargspec deprecation warning in Python 3
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