search

awsdocs / aws-cdk-guide

User guide for the AWS Cloud Development Kit (CDK).
Other
335 stars 222 forks source link

aws_cdk.aws_codebuild.PipelineProject: Inconsistent type translation JS/Python #294

Closed justin-ad closed 3 years ago

justin-ad commented 3 years ago

It seems that the auto translation being used to generate the documentation is littering the Python docs with JS specific type information. Please see https://docs.aws.amazon.com/cdk/api/latest/python/aws_cdk.aws_codebuild/PipelineProject.html

environment_variables (Optional[Mapping[str, BuildEnvironmentVariable]]) – Additional environment variables to add to the build environment. Default: - No additional environment variables are specified.

Why is the type for environment_variables specified as Option[Mapping[str, BuildEnvironmentVariable]])? In Python, this should be (Optional[dict{str: BuildEnvironmentVariable}]). What is Mapping? Python has no such type... this is a dict in Python.

ghost commented 3 years ago

This looks correct to me. Mapping is Python's abstract base class that all dictionary-like types belong to. See:

https://docs.python.org/3/library/collections.abc.html#collections.abc.Mapping

These abstract types are recommended for use in Python type hints. See:

https://docs.python.org/3/library/typing.html#generics

The CDK will accept any mapping type here, not just dict, and it doesn't need to be mutable, hence Mapping rather than MutableMapping.

This definitely isn't a leak from TypeScript/JavaScript. (The mapping type is called Object in those languages.)

I'll close this; feel free to re-open if you have further comments.

Your feedback helps us make the AWS CDK better for everyone. Keep it coming!