Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

python: data type field names are not capitalized correctly #578

Closed
eladb opened this issue Jul 2, 2019 · 1 comment · Fixed by #586
Closed

python: data type field names are not capitalized correctly #578

eladb opened this issue Jul 2, 2019 · 1 comment · Fixed by #586
Labels
bug This issue is a bug. language/python Related to Python bindings p0

Comments

@eladb
Copy link
Contributor

eladb commented Jul 2, 2019 •
edited
Loading

For example:

@jsii.data_type(jsii_type="@aws-cdk/aws-apigateway.DomainNameOptions", jsii_struct_bases=[_DomainNameOptions])
class DomainNameOptions(_DomainNameOptions):
    certificate: aws_cdk.aws_certificatemanager.ICertificate
    domainName: str
    """

I would have expected domain_name instead of domainName.

Furthermore, there is no ctor, so I don't get type completion when I am creating an instance. Would be nice to add a ctor for all the fields.
@eladb eladb added bug This issue is a bug. language/python Related to Python bindings labels Jul 2, 2019
@fulghum fulghum added the p0 label Jul 2, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Jul 4, 2019

Data types are a weird beast though. They're TypedDicts, not regular classes.

@rix0rrr rix0rrr mentioned this issue Jul 7, 2019
Merged
@eladb eladb closed this as completed in #586 Jul 7, 2019
eladb pushed a commit that referenced this issue Jul 7, 2019
@rix0rrr
feat(python): idiomatic capitalization for structs (#586)
51211a0 
This change addresses 4 issues:

- Structs now use idiomatic (snake_case) capitalization of fields
  (instead of JSII-inherited camelCase).
- IDE support -- replace TypedDict usage with regular classes. This
  makes it so that we can't use dicts anymore, but mypy support in
  IDEs wasn't great and by using POPOs (Plain Old Python Objects)
  IDEs get their support back.
- Structs in a variadic argument use to be incorrectly lifted to
  keyword arguments, this no longer happens.
- Stop emitting "Stable" stabilities in docstrings, "Stable" is implied.

In order to make this change, I've had to make `jsii-pacmak` depend on
`jsii-reflect`. This is the proper layering of libraries anyway, since
`jsii-reflect` adds much-needed--and otherwise
duplicated--interpretation of the spec.

Complete refactoring of `jsii-pacmak` to build on `jsii-reflect` is too
much work right now, however, so I've added "escape hatching" where
generators can take advantage of the power of jsii-reflect if they want
to, but most of the code still works on the raw spec level.

Added a refactoring where we load the assembly once and reuse the same
instance for all generators, instead of loading the assembly for every
generator. Assembly-loading, especially with a lot of dependencies, takes
a non-negligible amount of time, so this has the side effect of making
the packaging step faster (shaves off 100 packages * 3 targets * a
couple of seconds).

Fixes #537 
Fixes #577 
Fixes #578 
Fixes #588
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This issue is a bug. language/python Related to Python bindings p0
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(python): idiomatic capitalization for structs aws/jsii
3 participants
@fulghum @rix0rrr @eladb