Nothing Special   »   [go: up one dir, main page]
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

FIX: Ensure loadpkl returns a not None value #3020

Merged
merged 1 commit into from
Sep 6, 2019
Merged

FIX: Ensure loadpkl returns a not None value #3020

merged 1 commit into from
Sep 6, 2019

Conversation

oesteban
Copy link
Contributor
@oesteban oesteban commented Sep 6, 2019

Ref.: #3014 (comment)

Acknowledgment

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

@effigies
Copy link
Member
effigies commented Sep 6, 2019 •
edited
Loading

Can you rebase on master, to ensure this interacts safely with #3017 and #3019?

@effigies
Copy link
Member
effigies commented Sep 6, 2019

A question: Should we be able to pickle None? Or is that just never a useful thing to do?

@satra
Copy link
Member
satra commented Sep 6, 2019

Should we be able to pickle None? Or is that just never a useful thing to do?

yes. custom dictionary inputs to custom functions. or custom outputs from custom functions.

@satra
Copy link
Member
satra commented Sep 6, 2019

but never as a stand alone unit in nipype (results, nodes, and crashfiles are always not None)

@oesteban
Copy link
Contributor Author
oesteban commented Sep 6, 2019

@effigies, rebased

effigies
effigies approved these changes Sep 6, 2019
View reviewed changes
Copy link
Member
@effigies effigies left a comment

Choose a reason for hiding this comment

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

LGTM.

@effigies effigies added this to the 1.2.2 milestone Sep 6, 2019
@codecov
Copy link
codecov bot commented Sep 6, 2019 •
edited
Loading

Codecov Report

Merging #3020 into master will decrease coverage by <.01%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3020      +/-   ##
==========================================
- Coverage   64.18%   64.18%   -0.01%     
==========================================
  Files         342      342              
  Lines       43987    43990       +3     
  Branches     5547     5549       +2     
==========================================
+ Hits        28233    28234       +1     
- Misses      14638    14639       +1     
- Partials     1116     1117       +1
Flag Coverage Δ
#unittests 64.18% <50%> (-0.01%) ⬇️
Impacted Files Coverage Δ
nipype/utils/filemanip.py 76.1% <50%> (-0.26%) ⬇️

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 09d55ec...19ce2c1. Read the comment docs.

@effigies effigies merged commit 13a7556 into nipy:master Sep 6, 2019
@oesteban oesteban deleted the maint/3014-related branch September 6, 2019 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@satra satra satra approved these changes

@effigies effigies effigies approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@oesteban @effigies @satra