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

DOC: Add Python 2 statement to README, reference maintenance branch in CONTRIBUTING #3115

Merged
merged 1 commit into from
Dec 19, 2019

Conversation

effigies
Copy link
Member

Summary

Ref #3113.

List of changes proposed in this PR (pull-request)

  • Add a Python 2 statement to README
  • Mention maintenance branch in CONTRIBUTING

Acknowledgment

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

@effigies
Copy link
Member Author

This could probably be expanded, but I didn't want to bloat the CONTRIBUTING guide too much. Let me know what you think.

@emdupre @choldgraf Have you seen other repositories that are keeping a Python 2-compatible branch alive for a bit, and how they're directing people to this? A quick search through some major packages has brought up nothing.

@codecov
Copy link

codecov bot commented Dec 16, 2019

Codecov Report

Merging #3115 into master will increase coverage by 0.63%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3115      +/-   ##
==========================================
+ Coverage   67.23%   67.86%   +0.63%     
==========================================
  Files         294      295       +1     
  Lines       39245    39263      +18     
  Branches     5162     5172      +10     
==========================================
+ Hits        26385    26646     +261     
+ Misses      12186    11912     -274     
- Partials      674      705      +31
Flag Coverage Δ
#smoketests 51.23% <ø> (ø) ⬆️
#unittests 65.09% <ø> (+0.87%) ⬆️
Impacted Files Coverage Δ
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%) ⬆️
nipype/interfaces/freesurfer/preprocess.py 66.04% <0%> (+0.98%) ⬆️
nipype/interfaces/freesurfer/model.py 64.19% <0%> (+1.12%) ⬆️
nipype/interfaces/spm/base.py 87% <0%> (+2%) ⬆️
nipype/interfaces/freesurfer/utils.py 62.88% <0%> (+2.13%) ⬆️
nipype/interfaces/io.py 59.67% <0%> (+6.11%) ⬆️
nipype/interfaces/dcm2nii.py 64.78% <0%> (+15.96%) ⬆️
nipype/interfaces/freesurfer/base.py 78.44% <0%> (+21.55%) ⬆️
... and 2 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 388f140...eb16dcc. Read the comment docs.

@choldgraf
Copy link

I don't know if any specifically. I know matplotlib backports a lot of stuff but nothing that's unique to python 2. I think at this point many packages are just telling users they'll drop support soon (or already have). Maintaining a separate python 2 branch seems like something you could do if there were dedicated funding for it, but to me feels too much work with volunteer labor

@effigies
Copy link
Member Author

In my mind, we won't be backporting, just accepting PRs against the maint/1.3.x branch for people who need to fix bugs but can't use Py3.

If it becomes a serious burden, then we can consider a hard cutoff, but it seems unlikely given the overall activity level in nipype.

@effigies effigies mentioned this pull request Dec 19, 2019
11 tasks
@effigies
Copy link
Member Author

In the absence of outrage, I'll go ahead and merge this one.

@effigies effigies merged commit 03e30cc into nipy:master Dec 19, 2019
@effigies effigies deleted the enh/contributing_py2 branch December 19, 2019 23:15
@emdupre
Copy link
Contributor

emdupre commented Dec 20, 2019

I also haven't seen any other libraries with this strategy, but it makes sense to me ! 👍

@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.

3 participants