Larry Hastings added the comment:
So, not to yank your chain, but... I'm okay with checking this in. Yes, we're
already in beta, but ModuleSpec is brand new, and the sense I get is that this
use case is obscure even for ModuleSpec. The only installed base is beta 1
users, and given that
Eric Snow added the comment:
I'll commit it in a little while. Thanks.
--
versions: +Python 3.4 -Python 3.5
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
Roundup Robot added the comment:
New changeset a72a0e4dad20 by Eric Snow in branch 'default':
Issue #19927: Add __eq__ to path-based loaders in importlib.
http://hg.python.org/cpython/rev/a72a0e4dad20
--
nosy: +python-dev
___
Python tracker
Changes by Eric Snow ericsnowcurren...@gmail.com:
--
resolution: - fixed
stage: patch review - committed/rejected
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
Larry Hastings added the comment:
You broke buildbots. Please fix.
http://buildbot.python.org/all/builders/AMD64%20FreeBSD%2010.0%203.x/builds/1389
--
assignee: - eric.snow
priority: normal - high
resolution: fixed -
stage: committed/rejected - needs patch
status: closed - open
Larry Hastings added the comment:
Hmm, hard to see how you caused that with the path loader change. Still please
take a quick look.
I fired off another build to see if it was a transient error, but that'll take
a while.
--
___
Python tracker
Eric Snow added the comment:
I'll take a look. It could be something with #19713 or #19708 that also failed
there.
The other failing buildbot for those 3 changesets is
http://buildbot.python.org/all/builders/x86%20Windows7%203.x/builds/7800.
--
Eric Snow added the comment:
The windows buildbot failure looks like a race condition in a test unrelated to
my changes (see issue #20127). I'm looking at the FreeBSD failure now.
--
___
Python tracker rep...@bugs.python.org
Eric Snow added the comment:
Which passed on the subsequent run...
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
___
Python-bugs-list
Eric Snow added the comment:
The FreeBSD failure happened in test_threading (apparently), where it was the
last test to finish. In the passing run it finished 339/388 -- the seed was
different (1253928 vs. 5389019).
This does not seem to be related to my 3 changesets. I'm guessing it's one
Changes by Eric Snow ericsnowcurren...@gmail.com:
--
status: pending - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
___
Eric Snow added the comment:
I'm fine with this. Thanks, Larry, for your attentiveness and diligence.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
Larry Hastings added the comment:
1. Is this patch going to change best practice for working with ModuleSpec?
2. If we delayed it to 3.5, will users have to ignore it to work around the
deficiencies of the ModuleSpec implementation in 3.4?
I'm guessing the answer to both of these is well, no,
Nick Coghlan added the comment:
Yes, I think it will just make the third party idiom for testing that the
right module was imported to be to check spec.origin rather than comparing
specs directly. It's a nice-to-have, rather than something essential that
justifies breaking feature freeze.
Nick Coghlan added the comment:
That is, I think the answer to both your questions is actually Yes, but it
doesn't really matter due to the obscurity of the use case.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
Larry Hastings added the comment:
That's not how this works, Eric. I have to give you permission to add a new
feature, which I remind you I have yet to do.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
Eric Snow added the comment:
My bad, Larry. I guess I was reading between the lines too much. :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
Nick Coghlan added the comment:
That reminds me: I ended up working around this in the runpy tests by only
checking the loader type was correct in the module specs. With an improved
definition of equality for loaders, the runpy tests could be both
simplified *and* made more rigorous at the same
Eric Snow added the comment:
Yeah, it was while writing tests that I ran into this.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
___
Larry Hastings added the comment:
So can you tell me how this will make users' lives easier? I don't really
understand the issues involved. But the only concrete thing I've seen
mentioned is making testing easier, and that's not worth breaking feature
freeze over.
--
Eric Snow added the comment:
Right now say you have 2 module specs that are the same. The only difference
is that the 2 loaders are not the same instance (they were created separately
with the same arguments, ergo equal). The two specs will not compare as equal
even though they are equal.
Nick Coghlan added the comment:
Yeah, I think we can safely leave this to 3.5.
--
priority: high - normal
versions: +Python 3.5 -Python 3.4
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
Nick Coghlan added the comment:
Importer writers are already used to __loader__ being annoying, and comparting
specs for equality is unlikely to be a common thing (and easily worked around
by comparing spec.origin instead)
--
___
Python tracker
Eric Snow added the comment:
Unless there are objections, I'll commit this in the next day or two.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
Eric Snow added the comment:
Good point, Nick. Here's a patch that follows your recommendation on both
points.
--
Added file: http://bugs.python.org/file33072/issue19927-loader-eq.diff
___
Python tracker rep...@bugs.python.org
Changes by Eric Snow ericsnowcurren...@gmail.com:
Removed file: http://bugs.python.org/file33038/issue19927-loader-eq.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
Larry Hastings added the comment:
Brett, could you weigh in please?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
___
Brett Cannon added the comment:
I'm fine with the suggestions Nick made. While loaders are not technically
immutable (and thus technically probably shouldn't define __hash__), they have
not been defined to be mutable and mucked with anyway, so I have no issue if
someone breaks the hash of a
Eric Snow added the comment:
(my browser farted the half finished report into existence :P )
The __eq__() implementation of the path-based loaders in importlib is just the
stock one that compares object identity. So two that are effectively the same
compare unequal. This has a material
Changes by Eric Snow ericsnowcurren...@gmail.com:
--
Removed message: http://bugs.python.org/msg205515
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
___
Eric Snow added the comment:
Here's a patch.
--
keywords: +patch
stage: test needed - patch review
Added file: http://bugs.python.org/file33038/issue19927-loader-eq.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19927
Nick Coghlan added the comment:
There can be some interesting backwards compatibility consequences when adding
an __eq__ implementation to a class that was previously using the default ID
based __eq__:
- it becomes unhashable (unless you also add a suitable __hash__ definition)
- subclasses
32 matches
Mail list logo
