Static Typing profilers/utils.py #630
Conversation
|
|
taylorfturner
added
Work In Progress
taylorfturner
requested review from
taylorfturner,
JGSweets,
ksneab7,
micdavis and
tyfarnan
JGSweets
changed the title
Head branch was pushed to by a user without write access
tonywu315
changed the title
|
@tonywu315 FYI, I think your code is out of date. You will likely need to rebase this branch onto |
dataprofiler/profilers/utils.py
Outdated
| if stat1 is None or stat2 is None: | ||
| return [i if i is None else i.strftime("%x %X") for i in [stat1, stat2]] | ||
| elif stat1 != stat2: | ||
| diff = stat1 - stat2 | ||
| return ("+" if diff.days >= 0 else "-") + str(abs(diff)) | ||
| return "unchanged" |
There was a problem hiding this comment.
any reason for removing the
diff = find_diff_of_numbers(stat1, stat2)
and handling this here?
There was a problem hiding this comment.
find_diff_of_numbers() only takes in ints and floats. If I make it also take in datetime, then mypy complains because ints/floats can't be subtracted with datetime.
There was a problem hiding this comment.
Fair, should we fix numbers to take in any type that can have a subtraction operation? Not sure if that's actually feasible. Otherwise, can do as is but fix the above.
There was a problem hiding this comment.
Found this:
https://stackoverflow.com/questions/37669222/how-can-i-hint-that-a-type-is-comparable-with-typing
from __future__ import annotations
from abc import abstractmethod
from typing import MutableSequence, Protocol, TypeVar
class Comparable(Protocol):
"""Protocol for annotating comparable types."""
@abstractmethod
def __lt__(self: CT, other: CT) -> bool:
pass
CT = TypeVar("CT", bound=Comparable)
def comparison_sort(s: MutableSequence[CT]) -> None:
pass
comparison_sort([1, 2, 3]) # OK
comparison_sort([1.0, 2.0, 3.0]) # OK
comparison_sort(["42", "420", "2137"]) # OK
comparison_sort([1, 2, "3"]) # mypy error Could we alter Comparable name and abstractions to use __eq__ and __sub__. which would allow a datetime to go into diff_of_numbers?
Maybe that's not appropriate either bc it would take in anything with that.
There was a problem hiding this comment.
I added this and it seems to work fine. The only thing is that I need to make __sub__ return Any because subtracting datetime results in a timedelta.
| T = TypeVar("T", bound=Subtractable) | ||
|
|
||
|
|
||
| def find_diff_of_numbers(stat1: Optional[T], stat2: Optional[T]) -> Any: |
There was a problem hiding this comment.
I think that's fine for now, but in the log run we should try to avoid using Any where possible.
Ideally we could create a typing file we use that makes these descriptive declarations. As things like any or Optional[T] don't make sense to someone reading it without diving deep into the code.
Will approve though, a future state for which to strive.
No description provided.