Adding time map kml support #136
Conversation
| ) -> T: | ||
| ) -> Union[T, TimeMapKmlResponse]: |
There was a problem hiding this comment.
This is a very annoying change to me, cause TimeMapKmlResponse does meet the requirements of the typevar T, but the type checker can't figure that out.
I tried reworking this with abstract ABC classes, couldn't fully resolve all type issues. Maybe in the future I'll look into a bigger type rework for this, but that's an issue for a separate PR.
There was a problem hiding this comment.
I'd like to avoid such hacks
There was a problem hiding this comment.
yea me too, but I think to avoid it I'd need to make a bigger rework, so wanted to do it in a separate PR
There was a problem hiding this comment.
what's the problem with TimeMapKmlResponse cause it looks the same as other response classes?
There was a problem hiding this comment.
it's a very weird issue, the type checker can't resolve that TimeMapKmlResponse can fit into T.
This happens because I have a different parsing method for kml (because it's xml and not json). It's hard to explain, I could show you over a call
| # The stable version of fastkml is from 2021 and does not have types defined, nor | ||
| # a `py.typed` file. There are newer pre-release versions as new as 2024, but they | ||
| # seem to be undoccumented. | ||
| # TODO: Maybe port this into newer versions of fastkml for proper type checking support | ||
|
|
There was a problem hiding this comment.
Regarding this, the current version is 0.12. They have 1.0 alpha versions available, seems like they have been slowly developing 1.0 for ~3 years now. It seems pretty vastly different and not documented, so for now I'll keep using the stable version with type checking disabled.
No description provided.