Add IsFinite type #43
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
==========================================
+ Coverage 94.08% 94.25% +0.17%
==========================================
Files 2 2
Lines 169 174 +5
Branches 27 27
==========================================
+ Hits 159 164 +5
Misses 8 8
Partials 2 2 ☔ View full report in Codecov by Sentry. |
| StrType = TypeVar("StrType", bound=str) | ||
| _StrType = TypeVar("_StrType", bound=str) |
There was a problem hiding this comment.
I don't see any reason for us to export these type vars
| _NumericType = TypeVar('_NumericType', bound=Union[SupportsFloat, SupportsIndex]) | ||
| IsFinite = Annotated[_NumericType, Predicate(math.isfinite)] |
There was a problem hiding this comment.
Others like IsNotNull are maybe a bit less valuable since the predicate function would have to be lambda x: not math.isnull(x) which can't be easily recognized.
Zac-HD
left a comment
There was a problem hiding this comment.
Looks good! Should we document that math.isfinite is a useful specific predicate to recognise, or just leave that to implementors?
|
Yup this does need docs! |
| - `IsLower = Predicate(str.islower)` | ||
| - `IsUpper = Predicate(str.isupper)` | ||
| - `IsDigit = Predicate(str.isdigit)` | ||
| - `IsFinite = Predicate(math.isfinite)` |
There was a problem hiding this comment.
We document this as if we're just defining the annotation, but actually define the annotated type.
There was a problem hiding this comment.
Agreed, I kept the existing format, but I'm not sure if that was an affirmation, question, or suggestion?
There was a problem hiding this comment.
More of a thoughtless observation, really 😓
Coming back with some more thought, I'm concerned that we're misleading people a bit with the docs - as written, I would expect to write Annotated[float, IsFinite] rather than IsFinite[float] - but my understanding is that the latter is actually required given our implementation so far, right? If so, I think we should change the docs.
There was a problem hiding this comment.
Yes, or we can just change the = to : and put an example there:
- `IsFinite`: `Predicate(math.isfinite)`
These predicates are used as `x: IsFinite[float]` which is equivalent to `x: Annotated[float, Predicate(math.isfinite)]` (that is, `IsFinite` is just a generic type alias for your convenience).
No description provided.