fix(python): classproperty not working with type checkers #3694
Conversation
In some scenarios, the `@classproperty` decorator would not yield something that python type checkers would consider as a property. This attempts to add additional type hints to help type checkers discover that the returned object is a property descriptor. Fixes #3633
RomainMuller
force-pushed
the
rmuller/py-class-properties
branch
from
73dbb4e to
096ccc9
Compare
| @@ -164,7 +166,7 @@ def new_combined_struct(structs: Iterable[Type]) -> Type: | |||
| label = " + ".join(struct.__name__ for struct in structs) | |||
|
|
|||
| def __init__(self, **kwargs): | |||
| self._values: Mapping[str, Any] = kwargs | |||
There was a problem hiding this comment.
According to pyright (which is correct), this is not a valid location to have a type signature at...
| return super.__setattr__(self, name, value) | ||
| return super().__setattr__(name, value) |
There was a problem hiding this comment.
seems to be a harmless cosmetic change, but I'm curious; why make this change in this PR?
There was a problem hiding this comment.
The type signature of the previous chain would not allow pyright to determine the signature of __setattr__ and it complained about receiving 3 arguments when 2 were expected.
| for python_name, jsii_name in python_jsii_mapping( | ||
| struct | ||
| for python_name, jsii_name in ( | ||
| python_jsii_mapping(struct) or {} |
There was a problem hiding this comment.
nice catch...I imagine this is a consequence of us running pyright on this file?
| _instances: Mapping[Type[Any], Any] = {} | ||
| _instances: MutableMapping[Type[Any], Any] = {} |
There was a problem hiding this comment.
I can't find any references to this property in this PR...why make this change?
There was a problem hiding this comment.
The _instances value is mutated, and Mapping is immutable... This should always have been typed as MutableMapping but mypy was more forgiving than pyright here...
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
|
Merging (with squash)... |
|
Merging (with squash)... |
Corrects multiple issues with running `bin` scripts from python. These were discovered in the context of this `projen` issue: projen/projen#2103 - Corrects broken argument marshaling to binary scripts introduced in #3694 - Exposes exit code and stderr from script execution instead of failing silently Additionally, adds more robust test coverage of passing arguments to `bin` scripts. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
The
jsii.pythonsubmodule was not re-exported in__init__.py, whichmade type resolves not manage to access the.
@classpropertydecoratorand default to treating it as
Any.This adds some missing type annotations, runs
pyrighton the runtimelibrary and tests thereof (as
mypydid not identify this problem),and adds the missing export declaration.
Fixes #3633
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.