Skip to content

ENH: enable pickle protocol 5 support for python3.5#16439

Merged
charris merged 3 commits into
numpy:maintenance/1.18.xfrom
suquark:pickle5_py35
May 30, 2020
Merged

ENH: enable pickle protocol 5 support for python3.5#16439
charris merged 3 commits into
numpy:maintenance/1.18.xfrom
suquark:pickle5_py35

Conversation

@suquark

@suquark suquark commented May 30, 2020

Copy link
Copy Markdown

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.

This PR follows #16421

@suquark suquark changed the base branch from maintenance/1.8.x to maintenance/1.18.x May 30, 2020 00:58
@charris

charris commented May 30, 2020

Copy link
Copy Markdown
Member

That didn't work. Did you do the git rebase HEAD^^ --onto maintenance/1.18.x bit from the new branch? You should be able to undo whatever you did.

@suquark

suquark commented May 30, 2020

Copy link
Copy Markdown
Author

Yes, I did. But I choose the wrong target branch when creating the PR (maintenance1.8.x). Doesn't it work by changing it to 1.18.x?

@charris

charris commented May 30, 2020

Copy link
Copy Markdown
Member

It just snipped off the last two commits and applied them to the HEAD of the maintenance/1.18.x. When you made the PR and used 1.8.x it applied everything from there up to the HEAD of 1.18.x plus your two commits. I've made that error myself, if you catch it before hitting PR button you can change the base branch again to get it where it belongs.

@charris

charris commented May 30, 2020

Copy link
Copy Markdown
Member

I think the azure failures are because we need #16308.

EDIT: Keeping old branches working can be a chore :(

@charris

charris commented May 30, 2020

Copy link
Copy Markdown
Member

I'll make a PR for that and see if it fixes the branch. See #16441.

@Qiyu8 Qiyu8 added the 57 - Close? Issues which may be closable unless discussion continued label May 30, 2020
@suquark

suquark commented May 30, 2020

Copy link
Copy Markdown
Author

Thanks for your answer. But I still feel a little confused. I actually mean I choose the wrong branch on the Github (the branch for the PR to be merged in), but my own branch is still based on maintenance/1.18.x (the correct one). So then I just switched the Github branch to 1.18.x. It should work anyway, right? Otherwise this PR cannot pass conflicts check.

@charris

charris commented May 30, 2020

Copy link
Copy Markdown
Member

@Qiyu8 Please remove the tags, and add back the old ones, this is an active PR.

@charris

charris commented May 30, 2020

Copy link
Copy Markdown
Member

@suquark yes, it is OK now. Originally it had changes for more than 1000 files.

@Qiyu8 Qiyu8 removed the 57 - Close? Issues which may be closable unless discussion continued label May 30, 2020
@Qiyu8

Qiyu8 commented May 30, 2020

Copy link
Copy Markdown
Member

@suquark sorry for the misleading label, It's not common to see non-master PR, Hope that #16441 solves CI failure.

@charris

charris commented May 30, 2020

Copy link
Copy Markdown
Member

Close/open to restart tests.

@charris charris closed this May 30, 2020
@charris charris reopened this May 30, 2020
@charris charris merged commit 9939902 into numpy:maintenance/1.18.x May 30, 2020
@charris

charris commented May 30, 2020

Copy link
Copy Markdown
Member

Thanks @suquark .

@jakirkham

Copy link
Copy Markdown
Contributor

Discussing a patch release with this change in issue ( #16483 ).

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