Skip to content

Conversation

@znicholls
Copy link
Contributor

@znicholls znicholls commented May 31, 2019

Updates the use of collections in preparation for Python3.8, see python/cpython@c66f9f8 (and also avoids deprecation warnings when using iris in Python3.7).

@znicholls znicholls marked this pull request as ready for review May 31, 2019 01:01
@znicholls znicholls force-pushed the collections-deprecation-warning branch from e9a936d to b5c4d17 Compare May 31, 2019 01:02
@znicholls
Copy link
Contributor Author

Test failures appear to all be plotting and docs related so I think this is blocked for now

@znicholls znicholls changed the title Update in preparation for Python3.8 deprecation of importing ABCs from collections Blocked: Update in preparation for Python3.8 deprecation of importing ABCs from collections May 31, 2019
@znicholls
Copy link
Contributor Author

@DPeterK is my understanding correct that this is failing for reasons other than the changes made in the PR?

@rcomer
Copy link
Member

rcomer commented May 31, 2019

@znicholls you are right, all recent PRs against master have these errors. I believe these should be fixed once #3316 is merged, then we can all rebase!

@evertrol evertrol mentioned this pull request Jun 13, 2019
@znicholls znicholls force-pushed the collections-deprecation-warning branch from d0c2515 to a233d48 Compare June 15, 2019 04:08
@znicholls znicholls force-pushed the collections-deprecation-warning branch 2 times, most recently from e4af836 to f695009 Compare June 15, 2019 04:11
@znicholls znicholls changed the title Blocked: Update in preparation for Python3.8 deprecation of importing ABCs from collections Update in preparation for Python3.8 deprecation of importing ABCs from collections Jun 15, 2019
@znicholls
Copy link
Contributor Author

@rcomer are you the right person to ask for a review?

@rcomer
Copy link
Member

rcomer commented Jun 17, 2019

are you the right person to ask for a review?

@bjlittle will know the best person to nominate to review this.

@znicholls
Copy link
Contributor Author

@bjlittle are you able to review this one, I think it's pretty trivial?

@ajdawson ajdawson self-assigned this Jul 16, 2019
@ajdawson
Copy link
Member

This looks OK except for the change to collections.Counter which I believe is incorrect. Can you sort that out @znicholls and we can get this merged.

@znicholls
Copy link
Contributor Author

znicholls commented Jul 16, 2019

Should be sorted now cheers @ajdawson !

@ajdawson
Copy link
Member

Thanks @znicholls, would you mind squashing into a single commit? In case you don't know, you can squash these commits by doing an interactive rebase onto the commit before your first one git rebase -i b209642 and choose squash/fixup for every commit except the first. Let me know if you have questions.

@znicholls znicholls force-pushed the collections-deprecation-warning branch from 8537452 to 76cee31 Compare July 16, 2019 09:22
@znicholls
Copy link
Contributor Author

Assuming I haven't mucked up the header handling I think this should be good now

@pp-mo
Copy link
Member

pp-mo commented Jul 16, 2019

would you mind squashing into a single commit?

Just intrigued @ajdawson - why do you want @znicholls to do that, when you can squash on merge yourself : is it just for the contributor to write the commit message ?

@ajdawson
Copy link
Member

Just intrigued @ajdawson - why do you want @znicholls to do that, when you can squash on merge yourself : is it just for the contributor to write the commit message ?

Habit I suppose... I assume you are referring to the squash-and-merge button? In this case that would have been a sensible use of it.

@ajdawson ajdawson merged commit 4d06023 into SciTools:master Jul 16, 2019
@pp-mo
Copy link
Member

pp-mo commented Jul 16, 2019

@ajdawson Habit I suppose...

😁

@bjlittle bjlittle mentioned this pull request Aug 2, 2019
@bjlittle bjlittle mentioned this pull request Aug 9, 2019
@pp-mo pp-mo mentioned this pull request Aug 20, 2019
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.

5 participants