Static typing #5
Conversation
This change declares the type of `p` to accept either the list of mapping return type.
Means we don't lose the information about whether the param `odds` is a mapping or other collection, and therefore we don't need to early capture `keys`. Also, `_class` was unused, so removed, however we probably can support returning the same type of mapping as the user inputs.
This allows us to use the nicer union syntax, `A | B`.
Sequences have specific order, important as the output list is ordered the same as the input sequence.
…ities` Makes all arguments other than `odds` kw only, this drastically reduces the number of required overloads to type the signature properly. ```py # still works calculate_implied_probabilities([2.0, 2.0]) calculate_implied_probabilities(odds=[2.0, 2.0]) calculate_implied_probabilities([2.0, 2.0], full_output=True) ## also any other combination of passing arguments as keyword args remains the same # patterns of passing any arg other than `odds` as positional breaks calculate_implied_probabilibies([2.0, 2.0], 1000) calculate_impolied_probabilities([2.0, 2.0], 1000, 1e-12, True) ``` This encourages clarity at the callsite and also helps from a library perspective as it is easier to add new kw only args to an api without breaking downstream. Adds overloads and tests for combination of input types and value of `full_output` flag.
Allows the key type of returned dict to match the input key type.
Previously, and input of `{"home": 2.0, "away": 2.0}` would return type `dict[Any, float]`. This change allows that to return `dict[str, float]`.
Instead of returning a dict that loses the type of the implied probabilities, we can return an object that is generic on the implied probability return type. This means that as far as mypy is concerned, the following two are equivalent: ```py calculate_implied_probabilities([2.0, 2.0]) calculate_implied_probabilities([2.0, 2.0], full_output=True).implied_probabilities ```
|
Looks good so far. How about adding a def __getitem__(self, item):
return getattr(self, item)which I believe will help with backwards compatibility as calling code won't have to change how it accesses the fields |
Thats a good idea - with a deprecation warning, or call it a feature? |
|
I think we can call it a feature. If it turns out it needs to be removed at some point we can start the deprecation process |
Configures mypy to run on tests as well as library code. Refactors tests to deal with type errors caused by reused variable names inside the mono-test.
marked ready for review but I still need to implement this - will do asap. |
|
What do you think about the name of the object |
|
I like the way |
Went with this. This branch is pretty much ready to go AFAIC. LMK if you want to see any changes. Cheers. |
This PR makes a range of changes to support static typing inference of the return types from
calculate_implied_probabilities(). It:__future__.annotationsso we can use the nicer modern typing syntaxSequenceinstead ofCollectionfor the non-mapping input type, this implies that order is important as the output list order matches the input odds order.oddskw onlyBREAKING: makes output whenfull_output=Truea generic dataclass instead of adict- this allows us to preserve the type of the implied odds on the object. (now non-breaking for reads as we implemented__getitem__on the dataclass).All of the changes have been added in individual commits and I've put notes in the commit messages for each.
I understand that the breaking changes might be a bit aggressive, however given that you are still 0-ver - I figured I may as well throw them out there and see what you think. Happy to discuss and make any changes.
Marking the PR as draft as I've not done the CI updates, however code changes are ready for review.
Thanks!