-
Notifications
You must be signed in to change notification settings - Fork 17
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
🍄 events moved to gaze df #885
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
from __future__ import annotations | ||
|
||
import inspect | ||
import logging | ||
import warnings | ||
from collections.abc import Callable | ||
from copy import deepcopy | ||
|
@@ -30,12 +31,16 @@ | |
import polars as pl | ||
|
||
import pymovements as pm # pylint: disable=cyclic-import | ||
from pymovements.events.processing import EventGazeProcessor | ||
from pymovements.gaze import transforms | ||
from pymovements.gaze.experiment import Experiment | ||
from pymovements.utils import checks | ||
from pymovements.utils.aois import get_aoi | ||
|
||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class GazeDataFrame: | ||
"""A DataFrame for gaze time series data. | ||
|
||
|
@@ -547,6 +552,7 @@ def deg2pix( | |
pixel_origin: str = 'upper left', | ||
position_column: str = 'position', | ||
pixel_column: str = 'pixel', | ||
verbose: bool = False, | ||
) -> None: | ||
"""Compute gaze positions in pixel position coordinates from degrees of visual angle. | ||
|
||
|
@@ -883,6 +889,54 @@ def detect( | |
how='diagonal', | ||
) | ||
|
||
def compute_event_properties( | ||
self, | ||
event_properties: str | tuple[str, dict[str, Any]] | ||
| list[str | tuple[str, dict[str, Any]]], | ||
name: str | None = None, | ||
verbose: bool = True, | ||
) -> None: | ||
"""Calculate an event property for and add it as a column to the event dataframe. | ||
|
||
Parameters | ||
---------- | ||
event_properties: str | tuple[str, dict[str, Any]] | list[str | tuple[str, dict[str, Any]]] | ||
The event properties to compute. | ||
name: str | None | ||
Process only events that match the name. (default: None) | ||
verbose: bool | ||
If ``True``, print information about the progress. (default: True) | ||
Raises | ||
------ | ||
InvalidProperty | ||
If ``property_name`` is not a valid property. See | ||
:py:mod:`pymovements.events.event_properties` for an overview of supported properties. | ||
RuntimeError | ||
If specified event name ``name`` is missing from ``events``. | ||
If no events are available to compute event properties. Consider calling detect before. | ||
|
||
Returns | ||
------- | ||
pm.EventDataFrame | ||
Returns self, useful for method cascading. | ||
""" | ||
if self.events is None: | ||
raise RuntimeError( | ||
'No events available to compute event properties. ' | ||
'Consider calling detect before.', | ||
) | ||
|
||
if verbose is True: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To log the debug message one would have to set So probably the verbose parameter isn't needed after all in this method and the if condition can be updated to if logger.isEnabledFor(logging.DEBUG): to avoid creating the debug string if not needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the verbose thing was there just to be consistent with the Dataset method, but maybe we can remove it together with the logging code above and keep it simple; what do you think, should we drop the logging altogether? |
||
logger.debug(f'Processing events {event_properties} matching {name} \ | ||
for \n{self.events.frame.head()}') | ||
|
||
processor = EventGazeProcessor(event_properties) | ||
new_properties = processor.process( | ||
self.events, self, identifiers=self.trial_columns, name=name, | ||
) | ||
join_on = self.trial_columns + ['name', 'onset', 'offset'] | ||
self.events.add_event_properties(new_properties, join_on=join_on) | ||
|
||
def measure_samples( | ||
self, | ||
method: str | Callable[..., pl.Expr], | ||
|
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 needed a bit of time to think about this.
we probably shouldn't set this in the package, as a user might have a custom logging config defined. after importing pymovements their config would be overwritten.
this logic should actually be on the side of the user, so I would suggest to revert the changes in this file.
still, defining logging levels and other configuration aspects can form a small mini tutorial. this would be out of scope of this PR but feel free to create one in case you like.
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.
indeed, will revert
although I'm not sure it's overwriting the user logging config, this should be only at the package level; but I'll have to check; on the long run probably a
logging.ini
would be more appropriate