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: Python 2.7-3.7.1 compatible NonDaemonPool #2754

Merged
merged 4 commits into from Oct 25, 2018

Conversation

effigies
Copy link
Member

Fixes #2745.

Python 2.7, 3.4-3.7.0, and 3.7.1 all have different interpretations of Pool.Process.

In Python 2.7, it's a class:

class Pool(object):
    '''
    Class which supports an async version of the `apply()` builtin
    '''
    Process = Process

In Python 3.4-3.7.0, it's a method that calls self._ctx.Process:

class Pool(object):
    '''
    Class which supports an async version of applying functions to arguments.
    '''
    _wrap_exception = True


    def Process(self, *args, **kwds):
        return self._ctx.Process(*args, **kwds)

And now, in Python 3.7.1, it's a static method that takes a context:

class Pool(object):
    '''
    Class which supports an async version of applying functions to arguments.
    '''
    _wrap_exception = True


    @staticmethod
    def Process(ctx, *args, **kwds):
        return ctx.Process(*args, **kwds)

@effigies effigies added this to the 1.1.4 milestone Oct 25, 2018
@codecov-io
Copy link

codecov-io commented Oct 25, 2018

Codecov Report

Merging #2754 into master will decrease coverage by 3.44%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2754      +/-   ##
==========================================
- Coverage   67.44%   63.99%   -3.45%     
==========================================
  Files         340      338       -2     
  Lines       43164    43116      -48     
  Branches     5351     5348       -3     
==========================================
- Hits        29110    27594    -1516     
- Misses      13355    14444    +1089     
- Partials      699     1078     +379
Flag Coverage Δ
#smoketests ?
#unittests 63.99% <90.9%> (-0.81%) ⬇️
Impacted Files Coverage Δ
nipype/pipeline/plugins/legacymultiproc.py 56.72% <90.9%> (+0.17%) ⬆️
nipype/interfaces/nilearn.py 40% <0%> (-56.67%) ⬇️
nipype/utils/spm_docs.py 25.92% <0%> (-44.45%) ⬇️
nipype/interfaces/freesurfer/base.py 49.59% <0%> (-30.9%) ⬇️
nipype/utils/logger.py 59.7% <0%> (-29.86%) ⬇️
nipype/algorithms/rapidart.py 35.39% <0%> (-29.21%) ⬇️
nipype/interfaces/spm/base.py 58.41% <0%> (-29.05%) ⬇️
nipype/utils/provenance.py 55.73% <0%> (-28.99%) ⬇️
nipype/interfaces/fsl/model.py 55.26% <0%> (-25.35%) ⬇️
nipype/interfaces/dipy/setup.py 0% <0%> (-25%) ⬇️
... and 47 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 39675df...c9a787e. Read the comment docs.

@effigies
Copy link
Member Author

Relevant Travis tests: https://travis-ci.org/nipy/nipype/builds/446288741

@oesteban Any objections? I'm pretty sure the whole NonDaemonPool thing was something you needed, so want to make sure I'm not breaking anything subtly.

Copy link
Contributor

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

Good job!

@Atterratio
Copy link

Awesome! @effigies Thanks a lot its help me in my own project!

@effigies effigies deleted the fix/multiproc37 branch January 22, 2019 13:21
alvarolopez added a commit to ai4os/DEEPaaS that referenced this pull request Nov 7, 2019
Some Python versions replacing the standard Pool with our custom Pool
causes the following error: "AssertionError: group argument must be None
for now." [1] This change uses a different approach, solving this issue.

[1]: nipy/nipype#2754
alvarolopez added a commit to ai4os/DEEPaaS that referenced this pull request Dec 10, 2019
Some Python versions replacing the standard Pool with our custom Pool
causes the following error: "AssertionError: group argument must be None
for now." [1] This change uses a different approach, solving this issue.

[1]: nipy/nipype#2754
wjia7 added a commit to Signiant/MaestroOps that referenced this pull request Jan 7, 2020
alvarolopez added a commit to IFCA-Advanced-Computing/DEEPaaS that referenced this pull request Nov 17, 2021
alvarolopez added a commit to IFCA-Advanced-Computing/DEEPaaS that referenced this pull request Nov 18, 2021
alvarolopez added a commit to IFCA-Advanced-Computing/DEEPaaS that referenced this pull request May 13, 2022
alvarolopez added a commit to IFCA-Advanced-Computing/DEEPaaS that referenced this pull request May 13, 2022
alvarolopez added a commit to ai4os/DEEPaaS that referenced this pull request Jun 8, 2022
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