[issue23447] Import fails when doing a circular import involving an `import *`

[Brett Cannon]

Brett Cannon added the comment:

I talked the semantic change over with Eric at the core sprint and we are both 
iffy enough on the semantic change and how it will impact stuff that we don't 
feel comfortable accepting the patch. Sorry, Steven, but thanks for taking a 
stab at it.

--
assignee: brett.cannon -> 
nosy: +eric.snow
resolution:  -> rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Steven Barker]

Steven Barker added the comment:

Thanks for looking at the issue Brett.

I think you're right that your patch has incorrect semantics, since it doesn't 
save the value to the provided namespace if it had to go through the special 
path to find the name. I think my patch got that part right (though I 
definitely missed one PyUnicode_Check).

I've just noticed that our patches may not always do the right thing in all 
error situations. They may return -1 without setting an exception (or perhaps 
passing on a strange one from an internal call) if there's something weird 
going on like `__name__` not existing or not being a string.

I'm attaching a new version of my patch that moves the PyErr_Format call down 
to the later check for `value == NULL`. This handles both the basic case where 
nothing strange goes on but the requested name is not found, and the weird 
cases where unexpected stuff goes wrong. It will replace any errors raised by 
the earlier code with an ImportError. I think an ImportError is the appropriate 
exception for most error cases, but perhaps some internal errors should not be 
overwritten and more complicated error logic is needed.

I also notice that the code in the "import_from" function (from which I 
borrowed heavily for my patch) was changed in one of the 3.5rc patches to have 
new error handling (it raises ImportError more often I think). I don't see an 
obvious way to copy its new error logic to import_all_from, but my C coding 
skills are rather rusty so I could be missing an easy approach.

--
Added file: http://bugs.python.org/file40649/23447_4.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Brett Cannon]

Brett Cannon added the comment:

Just so Steven knows, I'm still thinking about whether I want to accept the 
patch (and waiting to see if anyone else has an opinion =). I did realize that 
the patch as it stands does not add the found module as an attribute on the 
package which is potentially one way the semantics subtly shift with this patch.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Brett Cannon]

Brett Cannon added the comment:

Here is Steven's patch touched up. I still have not decided if the semantic 
change is worth it. If anyone else has an opinion please feel free to speak up. 
All tests do pass, though, so it at least doesn't seem to directly break 
anything.

--
Added file: http://bugs.python.org/file40443/issue23447.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Brett Cannon]

Changes by Brett Cannon :


--
assignee:  -> brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Steven Barker]

Steven Barker added the comment:

I've managed to partially fix my build environment, so I can verify that my 
patch (attached previously) works as intended. I'm not completely sure that it 
doesn't break unrelated things as my build still has lots of failing tests 
(which all appear to be unrelated to this issue). Nothing obviously new breaks 
with the patch applied.

The test I added in the patch does fail (as expected) on the unpatched 
interpreter, but passes on a patched version (though I've just noticed that the 
attempt at catching the exception doesn't work, since the current import code 
raises an AttributeError rather than the ImportError I had expected). I don't 
believe any other tests failed anew on the patched code, but like I said, I've 
got a lot of unrelated test failures so I might be missing some subtle issues.

I'm attaching an updated version of the patch with better exception handling in 
the test code. The patch applies cleanly to the latest dev branch. I'd 
appreciate any reviews or other feedback.

--
Added file: http://bugs.python.org/file40415/23447_3.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Robert Collins]

Robert Collins added the comment:

Yep: The issue has no clear solution , e.g., no agreement on a technical 
solution or if it is even a problem worth fixing.

Brett is saying he doesn't consider this a bug.

Steven says he doesn't have time to push it forward.

Oh, I see there is a patch attached though (I wish the attachments were linked 
at the message that adds them as well as globally).

--
stage: needs patch - patch review
status: languishing - open

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23447
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Mark Lawrence]

Mark Lawrence added the comment:

Languishing? :)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23447
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Mark Lawrence]

Mark Lawrence added the comment:

Can someone reset the status and open fields, thanks.

--
nosy: +BreamoreBoy
versions: +Python 3.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23447
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Robert Collins]

Robert Collins added the comment:

reset.

--
nosy: +rbcollins
resolution: not a bug - 
stage:  - needs patch
status: closed - languishing

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23447
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Steven Barker]

Steven Barker added the comment:

I've finally gotten around to looking into this issue and it turns out that 
fixing from package import * to work with circular imports is pretty easy, as 
all that needs to be done is apply the same logic from the patch for issue 
17636 to the loop in the next function. I'll attach a patch that does this.

Unfortunately, my Windows build environment seems to be completely messed up at 
the moment and I'm getting link errors on everything in my python repo, so I've 
not been able to test the code at all. Since I have no idea when I'll actually 
have the time to learn what the hell's going wrong with MSVC, I though I'd 
submit my patch and perhaps somebody on a better OS can make sure it works 
properly. I've included a new test which should show the issue with circular 
imports (and verify that the rest of the patch fixes it, hopefully).

--
keywords: +patch
Added file: http://bugs.python.org/file40047/23447_2.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23447
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue23447] Import fails when doing a circular import involving an `import *`

[Brett Cannon]

Brett Cannon added the comment:

If you want to take a stab at it, Steven, go for it and I will review the 
patch, but as you pointed out this is such an edge case that I'm personally not 
going to worry about fixing it and still don't consider it a bug.

--
title: Relative imports with __all__ attribute - Import fails when doing a 
circular import involving an `import *`

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23447
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com