Skip to content

Fix circular references#44

Merged
sloria merged 1 commit intomarshmallow-code:devfrom
itajaja:fix-circular-references
Dec 31, 2015
Merged

Fix circular references#44
sloria merged 1 commit intomarshmallow-code:devfrom
itajaja:fix-circular-references

Conversation

@itajaja
Copy link
Copy Markdown
Contributor

@itajaja itajaja commented Dec 21, 2015

fixes #41

Comment thread apispec/LazyDict.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you pass *args and **kwargs to the super call, can this update call be removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I guess it was a remnant of another experiment, I guess I can remove the __init__ entirely

@sloria
Copy link
Copy Markdown
Member

sloria commented Dec 22, 2015

Thanks for the PR!

Just a few driveby comments for now. I will try to look at this more closely over the holidays

@itajaja
Copy link
Copy Markdown
Contributor Author

itajaja commented Dec 22, 2015

no problem! I don't know how bulletproof that lazy dictionary is, but it gets the job done. Another change that would involve breaking the apis would be to first declaring the schema through definition and then triggering the schema generation with some sort of generate function

@itajaja
Copy link
Copy Markdown
Contributor Author

itajaja commented Dec 29, 2015

Any chance you looked at this? I can look at the conflicts if you think the general idea is sound

@sloria
Copy link
Copy Markdown
Member

sloria commented Dec 30, 2015

@itajaja Apologies for the delay. The approach is sound; works as advertised.

Just a few nitpicks

  • Module names should be snake-case, so apispec.lazy_dict, not apispec.LazyDict.
  • A docstring for LazyDict would be helpful. It would be worth noting that it the lazy_dict module should be considered private API.

using a lazy loading dictionary, it's possible to delay the schema definition,
so that the references schema references are solved after the schemas are
registered with the  function
@itajaja itajaja force-pushed the fix-circular-references branch from 8e39c78 to fa1ec52 Compare December 30, 2015 09:42
@itajaja
Copy link
Copy Markdown
Contributor Author

itajaja commented Dec 30, 2015

added your nits, resolved conflicts and squashed 🍷

@sloria
Copy link
Copy Markdown
Member

sloria commented Dec 31, 2015

Thanks!

sloria added a commit that referenced this pull request Dec 31, 2015
@sloria sloria merged commit 7fff39c into marshmallow-code:dev Dec 31, 2015
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.

circular dependency triggers stackoverflow

2 participants