Typeinterval part1 #46080
Typeinterval part1 #46080
Conversation
|
Shameless plug :) PR based on the Microsoft-stubs: #44922 |
Ugh. I wrote that stub in the MS stubs, which is why I tried to then convert it over to use in pandas. Took me a long time to get this far! I guess I should close this? @twoertwein ? |
|
Sorry @Dr-Irv getting interval.py right is definitely a mess. Let's keep this PR open in case my PR is not the preferred solution. |
If we go this direction, then I still have to create a PR for the |
IMO baby steps is definitely OK with typing. |
pandas/io/excel/_odfreader.py
Outdated
| return stamp.time() | ||
| else: | ||
| self.close() | ||
| raise ValueError(f"Unrecognized time {str(cell)}") |
There was a problem hiding this comment.
If changing code in a typing PR, I have some questions...
does it change behavior?
has typing found a latent bug?
is a release note needed?
do we need some additional tests?
There was a problem hiding this comment.
Given that it is a time, the other solution is to just do a cast to Timestamp since we know the result won't be a NaTType . I will revert to that with a comment
@jbrockmendel created nearly all the stubs for the cython code last year and also created a stub for libinterval #41059
yep. from #41059 (comment)
|
|
thanks @Dr-Irv |
I would like to add
pandas/_libs/interval.pyi, and I have a tentative file for that, but when I started to do that, it uncovered issues in our existing typing, because currently,mypyseesIntervalasUnknown, which allows a bunch of things to pass.So this PR addresses that issue. The way to see the problems that were fixed by this PR is to change in
_typing.pyi:TO
A followup PR will add
pandas/_libs/interval.pyiand make changes to handle that.