fix(python): imports between subpackages are broken #1528
Conversation
Using absolute imports from a sub-package to another of the same root package causes cycles in the module loading machinery, resulting in obscure errors (typically, the loader complaining that the root module name is not defined). This changes the name generation mechanism so that relative imports are used to refer to sub-packages within a same root, as per the best practices. This allows the module loaded to flatten the loop and successfully load everything (granted, as always, there is no intractible loop between the sub-packages). In addition, this change makes the `._jsii` submodule take care of ensuring the current package's depencencies' assemblies are loaded up before loading it's own module. This alleviated the need to systematically import all dependencies from each sub-package, instead only loading up the packages that are effectively used within a module.
| @@ -1168,3 +1168,9 @@ def test_collection_of_interfaces_map_of_structs(): | |||
| def test_collection_of_interfaces_map_of_interfaces(): | |||
| for elt in InterfaceCollections.map_of_interfaces().values(): | |||
| assert getattr(elt, "ring") is not None | |||
|
|
|||
|
|
|||
| @pytest.mark.skip # Currently broken because submodule names aren't case-adjusted correctly :( | |||
There was a problem hiding this comment.
This is a pre-existing bug I'm merely shedding some light on. In order to fix this, I believe we'll need the kind of work in #1502 - specifically modeling the submodules information (including of dependencies) as part of the Assembly, so we can tell a PascalCased submodule name apart from a nested type's parent.
Technically, we are already loading some submodules (jsii_calc.composite for example), but those are of the "inline namespace" species that predated the submodules feature (as it was introduced in #1286), and it coincidentally avoids the buggy parts here...
RomainMuller
requested review from
a team
and removed request for
MrArnoldPalmer
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
...ii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/submodule/back_references/__init__.py
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
…r' into rmuller/redo-python-name-resolver
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Restore unconditional import of datetime, enum, and typing. While they're not necessary all the time, unconditional import doesn't hurt much.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Adds a `submodules` property on the `Assembly` structure, and carry it forward in the `dependencyClosure`, so the information can later be used to improve code generation for languages such as Python where the submodule structure is modeled as first-class entity that needs to be propertly dealt with. It can also help with properly adjusting the submodule names so they look more "native" in target languages, without facing problems when trying to generate type names in dependent packages. The forwarding of dependent submodules is not exercized in the current regression test suite because of a pair of other bugs (#1528, #1557) that need to be addressed before the generated Python code can successfully load. The last of those PRs to be merged will incldude the necessary test coverage.
Adds a `submodules` property on the `Assembly` structure, and carry it forward in the `dependencyClosure`, so the information can later be used to improve code generation for languages such as Python where the submodule structure is modeled as first-class entity that needs to be propertly dealt with. It can also help with properly adjusting the submodule names so they look more "native" in target languages, without facing problems when trying to generate type names in dependent packages. The forwarding of dependent submodules is not exercized in the current regression test suite because of a pair of other bugs (#1528, #1557) that need to be addressed before the generated Python code can successfully load. The last of those PRs to be merged will incldude the necessary test coverage. This change is necessary for these PRs to be able to fix their respective issues.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
RomainMuller
dismissed
eladb’s stale review
As discussed, dismissing to allow another reviewer to have final word
|
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)... |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Merging (with squash)... |
Commit Message
fix(python): imports between subpackages are broken (#1528)
Using absolute imports from a sub-package to another of the same root
package causes cycles in the module loading machinery, resulting in
obscure errors (typically, the loader complaining that the root module
name is not defined).
This changes the name generation mechanism so that relative imports are
used to refer to sub-packages within a same root, as per the best
practices. This allows the module loaded to flatten the loop and
successfully load everything (granted, as always, there is no
intractible loop between the sub-packages).
End Commit Message
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.