[FEAT] Refactor location as self-validation dataclass for validating coordinates - #2844
[FEAT] Refactor location as self-validation dataclass for validating coordinates#2844JoLo90 wants to merge 3 commits into
Conversation
115aa86 to
5e26160
Compare
|
My analysis of switching to a dataclass suggested that |
| "datetime.tzinfo object (including subclasses)" | ||
| @staticmethod | ||
| def _validate_value(value: numeric, name: str, lower: numeric, upper: numeric): | ||
| if not isinstance(value, numeric): |
There was a problem hiding this comment.
This is where I draw the line with regard to Python fundamentally being dynamically/duck typed and going no further than type hints to suggest the appropriate type (as opposed to checking the type explicitly). I think the range restriction validation is appropriate here, however.
|
Oh I forgot: In @markcampanelli's magical world, this dataclass (and so many others) would be frozen! Oh immutability, thou arst so hard to achieve in shared Python codebases. 😭 |
2 participants
Location.tz#1752docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.[WIP]
The PR has two commits; the first commit e8073d0 aims to solve #2843 moving the validation of latitude, longitude and altitude into the dataclass.
While working on the tz attribute, I realized that tz is not very useful (it belongs more to the times parameter than to the site), and its intent is not very clear IMO. So I created a second commit 115aa86, which also removes tz.
The two commits are maybe too much for a single PR (and we should deprecate before deleting), but the intent was to show how lightweight and cleaner Location could be.