Correct type annotations in Realtime #1960
Merged
Conversation
This file contains hidden or 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
1. Realtime superclass in leaflet is GeoJson (which is a subclass of featuregroup). In Folium I cannot make Realtime a subclass of GeoJson since GeoJson requires features to be present before rendering. I made it a subclass of FeatureGroup to more clearly document that features can be added to a Realtime layer. 2. The container parameter for Realtime cannot just be any `L.Layer`. It must be a `FeatureGroup` or something that allows adding features. I created type annotations to reflect this on the Folium side.
Conengmo
requested changes
Jun 14, 2024
Member
Conengmo
left a comment
Conengmo
left a comment
There was a problem hiding this comment.
Small comment about updating the docstring, good to go afterwards!
Conengmo
approved these changes
Jun 16, 2024
Member
Conengmo
left a comment
Conengmo
left a comment
There was a problem hiding this comment.
Don't know why that test failed, maybe a hickup and we should try rerunning it? It's approved anyway.
Member
Member
|
Keep merging things without worrying about that Windows failure. It is a problem with the dependencies in conda-forge and we are working to fix it upstream. |
Conengmo
reviewed
Jun 18, 2024
| @@ -1,4 +1,5 @@ | |||
| branca>=0.6.0 | |||
| fiona | |||
Member
There was a problem hiding this comment.
@hansthen this got merged with this added dependency in the requirements. I don’t think that’s right, since it’s not a required dependency. Could you maybe make a PR to revert this?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR corrects a few type annotations for the
Realtimeplugin.Realtime superclass in leaflet is GeoJson (which is a subclass of featuregroup). In Folium I cannot make Realtime a subclass of GeoJson since GeoJson requires features to be present before rendering. I made it a subclass of FeatureGroup to more clearly document that features can be added to a Realtime layer.
The container parameter for Realtime cannot just be any
L.Layer. It must be aFeatureGroupor something that allows adding features.I created type annotations to reflect this on the Folium side.