Refactor geo_interface to use a standardized mixin. #105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| ref: https://gist.github.com/sgillies/2217756#__geo_interface | ||
| """ | ||
| return self.dict(exclude_unset=True) |
|
🤯 thanks for a great PR 🙏 |
vincentsarago
approved these changes
Feb 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I am changing
__geo_interface__methods. Because the models represent geojson already returning themselves as a dict is all that is necessary. This implementation aligns with what other libraries are doing as well as the discussion in __geo_interface__ specification and implementation #96. The original gist pre-dates the current geojson spec and has some slight inconsistencies, but the intent is to match geojson.How I did it
__geo_interface__to use on the models.self.dict()for the response.How you can test it
__geo_interface__and then produces the same WKT as our WKT functions we know it is producing usable data in__geo_interface__Related Issues
I prefer consolidating code where I can, so I did it as a mixin. If it is preferable, we could just have the function on each of the models directly rather than a mixin. Since mixins do add to complexity of understanding. I was using this as a way to learn the right way to do a mixin like this in pydantic and keep everything happy. So it has at least served that purpose.