-
Couldn't load subscription status.
- Fork 50
Typing of core classes #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Typing of core classes #96
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay this is kind of crazy because I was about to make a pull request for this same reason 🤯 Thanks for doing this!
pyannote/core/annotation.py
Outdated
|
|
||
| def itertracks_with_labels(self) -> Iterator[SegmentTrackLabel]: | ||
| """Typed version of :func:`itertracks`(yield_label=True)""" | ||
| return self.itertracks_with_labels() # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is what you meant:
| return self.itertracks_with_labels() # type: ignore | |
| return self.itertracks(yield_label=True) # type: ignore |
However—while I'd be fine with having itertracks_with_labels and itertracks_without_labels—a better approach to getting typed itertracks might be using overload like I did here:
@overload
def itertracks(self, yield_label: Literal[False] = ...) -> Iterator[Track]: ...
@overload
def itertracks(self, yield_label: Literal[True]) -> Iterator[LabeledTrack]: ...
@overload
def itertracks(self, yield_label: bool) -> TrackIterator: ...(Though this would obviously need to be slightly altered to use your NamedTuples). This has the added benefit of working with existing code. The same thing can probably be done with Timeline.crop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comments @evfinkn
I have tried overloaded typing with bool constants in the past, and then stumbled upon the note that overloading is only allowed for types, not values. However, I did not consider your approach of using Literal[False]. I will have to see if this works with pylance. if it does the overload approach is far more elegant 😀
edit: tested it and it works! Changed in 230cf3b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few months later, here I am looking at the PR.
That's great thanks.
Any chance you could the same for Timeline.crop_iter with and without returned mapping?
Co-authored-by: Evan Finken <evan@finken.name>
|
FYI -- a bit busy right now but I did notice this PR. I think |
This PR should make working with
Annotationeasier for those using type checkers.In my own project I neede to wrap
Annotationwith another class and add ugly type-checked code like this:Now typed versions of
yield_label=Trueandyield_label=Falseexist. Type checkers do not allow to "set the type" by parameter value.Details
SegmentTrackandSegmentTrackLabelas return types for itertracks in annotation.pyNotes on NamedTuples
SegmentTrack(NamedTuple)andSegmentTrackLabel(NamedTuple)are fully compatible with Tuple[Segment, TrackName]andTuple[Segment, TrackName, Label], i.e. they can be deconstructed viaa,b,c = ...and accessed withtup[0],tup[1]In addition the fields can be accessed via dot-notation. This also works type-checked for the union type ofSegmentTrackandSegmentTrackLabel` for the common fields.