Skip to content

Conversation

@picnixz
Copy link
Member

@picnixz picnixz commented Oct 11, 2025

This is based on the observation by @Locked-chess-official about obj.__dir__ returning the unsorted and unfiltered list of names that could be returned by dir. I misassumed that obj.__dir__ had some magic applied onto it so thanks for the idea.

cc @ambv @devdanzin

StanFromIreland

This comment was marked as resolved.

@picnixz picnixz marked this pull request as draft October 11, 2025 10:22
@picnixz

This comment was marked as resolved.

@picnixz

This comment was marked as resolved.

@picnixz

This comment was marked as resolved.

@picnixz picnixz force-pushed the fix/traceback/suggestions-139933 branch 4 times, most recently from a3477e3 to 860ab2d Compare October 11, 2025 10:44
@Locked-chess-official

This comment was marked as resolved.

@picnixz

This comment was marked as resolved.

@picnixz picnixz force-pushed the fix/traceback/suggestions-139933 branch from 860ab2d to 2478e05 Compare October 11, 2025 11:11
@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

Using a isinstance(obj, type) test would not be good in the first place because we could try to get the suggestions of a metaclass itself, so we should first try obj.__dir__ anyway.

@picnixz picnixz marked this pull request as ready for review October 11, 2025 11:14
@Locked-chess-official
Copy link
Contributor

Add this test:

import sys
sys.modules['intmodule'] = int
from intmodule import ...

The name is pending.

@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

sys.modules['intmodule'] = int

I wouldn't count this as a legitimate use. Imports should only return modules. I don't want to support a bad sys.modules here.

@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

Or did I misunderstand and you want to test that there is indeed nothing good here?

@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

Note: the exception would occur in mod = __import__(exc_value.name) because it would be int in this case. So I don't think we should do anything more here.

@Locked-chess-official
Copy link
Contributor

Locked-chess-official commented Oct 11, 2025

Note: the exception would occur in mod = __import__(exc_value.name) because it would be int in this case. So I don't think we should do anything more here.

If the modulename in sys.modules, the exception will be raised only if it is None and message is "ModuleNotFoundError: import if $name halted, None in sys.modules".

@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

I don't understand. Your example already fails at the __import__(exc_value.name), not when computing the suggestion:

import sys
sys.modules["foo"] = int
from foo import rea

This will raise an ImportError and we won't have a suggestion here at all. For the runtime completion suggestions, this is something different. The ImportError is:

ImportError: cannot import name 'rea' from 'int' (unknown location)

@Locked-chess-official
Copy link
Contributor

If it is exception in __import__, it is strange. I'll take more time to read the code.

Screenshot_2025_1011_204835_ru.iiec.pydroid3.png

@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

That's because you're importing intmodule. exc_value.name here is int, not intmodule (and that's because it's not a module I guess).

EDIT: when you write from intmodule import X, what happens is that we try to import sys.modules["intmodule"].__name__ which is int and not intmodule.

For instance:

class FakeMeta(type):
    __name__ = "re"
 
class FakeName(metaclass=FakeMeta):
    pass

import sys
sys.modules["fake"] = FakeName

try:
    from fake import escape
except ImportError as exc:
    print(exc.name)       # prints "re"
    print(exc.name_from)  # prints "escape"

IOW, when writing from X import Y, the X is not relevant as we actually use the name given by the module. This is so that we can also write sys.modules[fancy_name] = types.ModuleName("other_fancy_name").

@Locked-chess-official
Copy link
Contributor

If it is because that, the question is exist:
Screenshot_2025_1011_210350_ru.iiec.pydroid3.png
If the name is the class name?
If the module name "int" is strange, the question will also occurs on the custom class.

@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

Ok, so this happens when we reuse the same name. But I don't think it's ok to mess things like that. My point is: what do you want me to do here? the behavior is somehow correct but if we want to have the same behavior for from fake import rea when sys.modules["fake"] = int and sys.modules["int"] = int, then I don't think it's a good idea. We could check if sys.modules[exc_value.name] already exists without trying to do __import__ but this is really for niche cases. We should not support random classes in sys.modules except None.

@Locked-chess-official
Copy link
Contributor

It is a behavior change. I think that it is needed to be recorded.

@python python deleted a comment from Locked-chess-official Oct 11, 2025
@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

(I deleted a duplicated comment).

What do you mean it's a behavior change? what is changing from before? I did document the change but I will not document a change affecting an incorrect usage of sys.modules.

@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

I think I understand what you mean. But since __import__ is already failing when trying to import the fake module, this is something that I cannot change in this PR (it seems something different). I'll try to bisect this.

@picnixz picnixz force-pushed the fix/traceback/suggestions-139933 branch from 2478e05 to 5dfb399 Compare October 11, 2025 13:50
@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2025

Ok, I finally understood what you wanted to tell me. Indeed, with my PR, if by chance we store some weird object when importing, then we would indeed have a change of behavior. Now, the existing behavior in 3.13 and 3.14 is "working" but I would say that we're in the garbage-in garbage-out situation where, by chance, it also works.

I don't know if it's preferrable to keep the behavior or actually fix it. Because I don't think it's actually correct to suggest something when the module to import is a class and not a module. OTOH, modules can be simply considered as special namespaces and someone could use classes to emulate singletons as we have for modules (something that is a namespace-like with some internal state emulated as class variables).

Anyway, for now I've kept the existing behavior although I think it's partially incorrect.

@picnixz picnixz force-pushed the fix/traceback/suggestions-139933 branch from 5dfb399 to 652970c Compare October 11, 2025 13:53
@picnixz picnixz requested a review from ambv October 11, 2025 14:00
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.

3 participants