Centralize add_segment and segments to BaseExtractor#4462
Centralize add_segment and segments to BaseExtractor#4462alejoe91 wants to merge 21 commits intoSpikeInterface:mainfrom
add_segment and segments to BaseExtractor#4462Conversation
| return len(self._segments) | ||
|
|
||
| def get_parent(self) -> BaseExtractor | None: | ||
| def get_parent(self) -> "BaseExtractor | None": |
There was a problem hiding this comment.
Hello. If you keep from __future__ import annotations you don't need to use string literals for self-referencing types. I think this is preferred, but might just be a style choice. At Python 3.11 we get a new type Self which is now the modern way to do self referencing.
There was a problem hiding this comment.
Ah nice! I would say we keep the string notation and get rid of future annotations. The main reason is that the __future__ import doesn't actually do any check, so we had a lot of bad typing (e.g., lists of | instead of Literal). This enforces us to be cleaner ;)
There was a problem hiding this comment.
Once we support python only great than 3.14 we can drop the strings again I think.
| # we remove the annotation if it exists | ||
| _ = self._annotations.pop("name", None) | ||
|
|
||
| @property |
There was a problem hiding this comment.
Maybe for typing we could in BaseRecording for example:
@property
def segments(self) -> list[BaseRecordingSegment]:
return self._segments # type: ignore[return-value]As that will enable the analysis of base recording sgements methods that we call on introspection with vscode and ohter tools. Same for bas sorting.
There was a problem hiding this comment.
Also, can we make this private? what is the reason for making this public?
3 participants
and fix a bunch of typing errors
Pulled out of #4216