Skip to content

Conversation

@suquark
Copy link

@suquark suquark commented May 29, 2020

Since the pickle5 library has enabled pickle protocol 5 support for python3.5 (https://github.com/pitrou/pickle5-backport, pitrou/pickle5-backport#15), it is reasonable to relax the constraints to support pickle protocol 5 under python3.5.

@seberg
Copy link
Member

seberg commented May 29, 2020

Seems reasonable. If we want to put this in, it has to be in the next few days, since it is only useful for 1.19, after that 3.5 will be dropped and there is no further 1.18 release planned right now.

@seberg seberg added 01 - Enhancement 09 - Backport-Candidate PRs tagged should be backported labels May 29, 2020
@seberg seberg added this to the 1.19.0 release milestone May 29, 2020
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label May 29, 2020
@charris charris removed this from the 1.19.0 release milestone May 29, 2020
@charris
Copy link
Member

charris commented May 29, 2020

Python 3.5 is already dropped in 1.19, we couldn't support f-strings otherwise :) It is too bad that the 3.5 backport wasn't available earlier. If you really, really need it for 3.5 you might try patching a 1.18 source release and compiling yourself.

@charris
Copy link
Member

charris commented May 29, 2020

If you want to make this PR against the 1.18 branch I'll put it in just so it is there, but I don't expect to make another release of that branch. Could maybe make a source release if that would be helpful.

@suquark
Copy link
Author

suquark commented May 29, 2020

Oops, I did't know numpy 1.19 has dropped Python3.5 support. It is totally ok for me to put it in the 1.18 branch (even without another release).

@seberg
Copy link
Member

seberg commented May 29, 2020

@charris right. I think the release notes may need to be clarified a bit. It says of Python versions <3.5

@charris
Copy link
Member

charris commented May 29, 2020

I think the release notes may need to be clarified a bit.

Got it, thanks.

@charris charris changed the base branch from master to maintenance/1.18.x May 29, 2020 14:21
@charris charris changed the base branch from maintenance/1.18.x to master May 29, 2020 14:22
@seberg
Copy link
Member

seberg commented May 29, 2020

@suquark happy to put this in, there is one test related to importerror when using protocol 5 but its not available. Could you modify that as well just to be sure?

@jakirkham
Copy link
Contributor

@charris right. I think the release notes may need to be clarified a bit. It says of Python versions <3.5

Came to do the same thing as @suquark and also was confused by this release note. Was going to submit a fix, but am now a bit perplexed by how release notes work. Where should we fix this?

@charris
Copy link
Member

charris commented May 29, 2020

@jakirkham The PR should be against maintenance/1.18.x, which is the last branch to support 3.5. If there is enough interest I could make another release of that branch. The easiest thing to do might be to close this PR, rebase on maintenance/1.18.x and resubmit.

@suquark You shouldn't be working in the master branch anyway. For two commits I'd do something like this:

$ git checkout -b <new branch>
$ git rebase HEAD~2 --onto maintenance/1.18.x
$ git checkout master
$ git reset --hard HEAD~2
$ git push -f origin HEAD

Which will clean up your master branch. Then push your new branch and open a PR.

@jakirkham
Copy link
Contributor

Sorry Charles was meaning how to fix the release notes.

It seems Siyuan already has this fix in hand.

@charris
Copy link
Member

charris commented May 29, 2020

was meaning how to fix the release notes.

NumPy 1.19.0 is still in the rc stage, so the release notes aren't finalized yet. I'll take care of it in the rc2.

@suquark suquark changed the title ENH: enable pickle protocol 5 support for python3.5 [Deprecated] ENH: enable pickle protocol 5 support for python3.5 May 30, 2020
@suquark suquark closed this May 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants