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
[WIP [ENH] metadata attribute for Annotations #12213
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
mne/annotations.py
Outdated
metadata : dict | array-like | ||
Metadata for the annotation. Can be a dict or an array-like | ||
object of dicts. If an array-like object, must be the same length | ||
as ``onset``. |
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.
with this we'd be exposing metadata
to the user --> weren't we discussing keeping it private, and having dedicated functions populate / add / use / change this data?
If we make it public here (exposing it in the docstring), then I think we might as well make it completely public 🤔
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.
with this we'd be exposing
metadata
to the user --> weren't we discussing keeping it private, and having dedicated functions populate / add / use / change this data?If we make it public here (exposing it in the docstring), then I think we might as well make it completely public 🤔
good point. That's inconsistent. We can also not allow to append Annotations with metadata manually. And behind the scenes then just do:
self._metadata = np.append(self._metadata, {})
I haven't checked, yet, whether .append()
is used somewhere in code, though.
update: just had a chat with @drammock and @britta-wstnr , and it looks like we'll approach this in a fundamentally different way. So please no one spend too much time with reading & commenting what will be outdated soon. |
…ioe/mne-python into feat/issue-12208/annotmetadata
for more information, see https://pre-commit.ci
# We need this, bc if we create Annotations from events which are not sorted, | ||
# and we then directly query for the according IDs, we cannot do this simply by | ||
# index as the Annotations will already on creation be sorted. | ||
def _get_id(self, onset, duration, description): |
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'm afraid that this is far from optimal and might be slow once there are many annotations.
Supposed to fix #12208 .
I want to add a private property
_metadata
tomne.annotations.Annotations
which stores a (list of)dict()
that can be used to pass along additional data with an annotation (for example, eye tracking realted info).[ This is WIP. Just started this PR to check back whether this goes into the right direction before I refactor the whole class. ]