microsoft / pyright

Static Type Checker for Python
Other
13.37k stars 1.46k forks source link

Does not detect certain sets in Pyomo #4956

Closed alexchandel closed 1 year ago

alexchandel commented 1 year ago

Describe the bug The open-source optimization modeling language Pyomo declares certain globals in a nontraditional manner, e.g. pyomo.core.base.set.NonNegativeReals. Pylance/Pyright reports a false positive "unknown import symbol" when you try to import them.

To Reproduce

from pyomo.environ import Binary, NonNegativeReals

These two symbols are defined here but Pylance does not recognize this.

Expected behavior Ideally, Pylance should detect these. Maybe it requires better type hints?

Screenshots or Code See above, with Pyomo 6.5.0.

VS Code extension or command-line Pylance v2023.4.20

erictraut commented 1 year ago

This would need to be fixed in the library. It's manipulating module namespaces in a way that's too dynamic for a static type checker to understand. The maintainers of the library would need to provide a static declaration, perhaps under an if TYPE_CHECKING conditional.

erictraut commented 1 year ago

This library is not marked as "py.typed". If you disable the useLibraryCodeForTypes configuration option, pyright will assume that all imports from a non-typed library are Unknown. This will eliminate the false positive error, but it will also eliminate all type checks (and language server features) that involve types imported from this library.

alexchandel commented 1 year ago

I attempted to add type hints for these types, but unfortunately they requires intersections which Python does not currently have. In particular, they need to be declared with GlobalSetBase & RangeSet.

Are you working on the PEP for intersections?

erictraut commented 1 year ago

I'm monitoring the work on the PEP for intersections but trying not to get sucked into it. It will involve a massive amount of work for relatively little gain. There are many other higher-priority investments we could collectively make to the type system and tooling that would have a much bigger impact for Python developers. For that reason, I don't think this is the right time to introduce intersections. Even if we could get a PEP ready for acceptance and get the typing ecosystem to rally around it (a tall order — and unlikely to happen anytime soon), it will take two or more years for other type checkers like mypy to support it. I can point to many examples over the past four years where this has been the case. So if you're relying on the addition of intersections, you're going to wait a long time.

There are simple and effective workarounds for the lack of intersection types. If GlobalSetBase and RangeSet are protocols, you can define a new protocol that includes elements of both subtypes. If these are nominal types, you could define a dummy subclass that derives from both classes and use that as the return type.

alexchandel commented 1 year ago

If GlobalSetBase and RangeSet are protocols, you can define a new protocol that includes elements of both subtypes. If these are nominal types, you could define a dummy subclass that derives from both classes and use that as the return type.

They're classes. The actual type of those values is a dynamically created class that's a subclass of GlobalSetBase and RangeSet. I want to commit the type hints to the upstream repo itself, so I don't think I can use a dummy subclass.

Even if we could get a PEP ready for acceptance and get the typing ecosystem to rally around it (a tall order — and unlikely to happen anytime soon), it will take two or more years for other type checkers like mypy to support it. I can point to many examples over the past four years where this has been the case. So if you're relying on the addition of intersections, you're going to wait a long time.

Would it be possible to preemptively add intersections to Pyright? Pyright seems to be in a much better place than mypy, and a working example would motivate the usefulness of the feature.

erictraut commented 1 year ago

so I don't think I can use a dummy subclass.

You could define a protocol class that defines the interface that comprises the intersection of GlobalSetBase and RangeSet.

Would it be possible to preemptively add intersections to Pyright?

No, that would be ill advised. For a feature as complex as intersections, there must first be agreement on how they work. If pyright were to go ahead and implement this ahead of an agreed-upon spec and then later a spec emerged, there would undoubtedly be backward compatibility issues, which wouldn't serve anyone well.

Also, the amount of work involved in adding support for generalized intersections is immense. I'd rather spend my limited time on investments that will have much bigger impact.