Skip to content

Conversation

@dansanduleac
Copy link
Contributor

@dansanduleac dansanduleac commented Jul 19, 2018

Before this PR

We import references to other conjure objects eagerly, i.e. we import the class directly, like so:

from .package.MyType import MyType

but if MyType is an object/union whose fields end up referencing back to MyType, python will never succeed in importing this way, because the imports will form a circular dependency.

After this PR

We place relative imports (that start with .) at the bottom of the file, such that if they cause a recursive import of an attribute from inside the module currently being loaded, it would have been defined already.
This solution has been identified thanks to this recommendation found online.

@dansanduleac dansanduleac changed the title [WIP] [fix] Support recursive object definitions [fix] Support recursive object definitions Jul 19, 2018
@dansanduleac dansanduleac requested review from ahggns and ferozco July 19, 2018 19:37
@ferozco
Copy link
Contributor

ferozco commented Jul 20, 2018

Unfortunately the modules already re-expose the types so that from ..product import StringExample is actually importing the class StringExample. You would have to introduce the concept of namespace imports so that you could do something like import ..product.StringExample as foo

@dansanduleac dansanduleac requested a review from iamdanfox July 20, 2018 12:23
@dansanduleac
Copy link
Contributor Author

dansanduleac commented Jul 20, 2018

Yep, @ferozco - rewrote this as we discussed earlier.

ahggns pushed a commit that referenced this pull request Jul 20, 2018
@ferozco
Copy link
Contributor

ferozco commented Jul 21, 2018

Closing in favour of #12

@ferozco ferozco closed this Jul 21, 2018
derenrich pushed a commit to derenrich/conjure-python that referenced this pull request Jan 8, 2019
@ferozco ferozco deleted the ds/no-from-imports branch June 4, 2019 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants