Skip to content

Fix some Python 3 issues flagged by the -3 flag#296

Merged
pganssle merged 1 commit into
dateutil:masterfrom
rowillia:master
Oct 12, 2016
Merged

Fix some Python 3 issues flagged by the -3 flag#296
pganssle merged 1 commit into
dateutil:masterfrom
rowillia:master

Conversation

@rowillia

@rowillia rowillia commented Oct 7, 2016

Copy link
Copy Markdown
Contributor

I've been working through looking at adopting Python 3, and I've been utilizing the -3 flag to help idenitfy issues with Python 3.

Here's a quick write up I did last week:

https://gist.github.com/rowillia/c0feed97c1863b2d8e5a3ed73712df65

The -3 flagged a few issues in dateutil worth fixing, namely around eq shadowing hash in PY3.

Fixing these issues will help make the -3 flag less noisey for folks running dateutil in production.

I've been working through looking at adopting Python 3, and I've been utilizing the -3 flag to help idenitfy issues with Python 3.

Here's a quick write up I did last week:

https://gist.github.com/rowillia/c0feed97c1863b2d8e5a3ed73712df65

The -3 flagged a few issues in dateutil worth fixing, namely around __eq__ shadowing __hash__ in PY3.

Fixing these issues will help make the -3 flag less noisey for folks running dateutil in production.
@rowillia

Copy link
Copy Markdown
Contributor Author

@pganssle Let me know if you've got any questions on this!

@pganssle

Copy link
Copy Markdown
Member

@rowillia Yes, sorry for the delay, it's been some time since I've had time to work on dateutil stuff.

I guess the issue is that classes default to hashable in Python 2, but to non-hashable in Python 3? I suppose if it hasn't come up already in Python 3 compatibility already, it's not a behavior too many people are relying on.

I would normally say we should add some tests, but I don't know that we want to signal that this behavior is definitely permanent. I could see the some possibility for adding a hash function in to the time zone objects, at least, at some point in the future, so I'm just going to merge as is.

@pganssle
pganssle merged commit 6794a73 into dateutil:master Oct 12, 2016
@pganssle pganssle added this to the 2.6.0 milestone Oct 12, 2016
@pganssle pganssle mentioned this pull request Nov 6, 2016
@mrigor mrigor mentioned this pull request Jun 2, 2017
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.

2 participants