Skip to content
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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/pymovements/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
# SOFTWARE.
"""Provides top-level access to submodules."""
import logging
import os

from pymovements import _version
from pymovements import datasets
from pymovements import events
Expand All @@ -44,7 +47,6 @@
from pymovements.measure import SampleMeasureLibrary
from pymovements.stimulus import text


__all__ = [
'Dataset',
'DatasetDefinition',
Expand Down Expand Up @@ -79,3 +81,7 @@
]

__version__ = _version.get_versions()['version']

LOG_LEVEL = os.environ.get('LOG_LEVEL', 'INFO')
LOG_CONFIG = '[%(levelname)s] %(asctime)s %(name)s:%(lineno)d - %(message)s'
logging.basicConfig(level=LOG_LEVEL, format=LOG_CONFIG)
Copy link
Contributor

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.

Copy link
Author

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

20 changes: 3 additions & 17 deletions src/pymovements/dataset/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
from pymovements.dataset.dataset_paths import DatasetPaths
from pymovements.events.frame import EventDataFrame
from pymovements.events.precomputed import PrecomputedEventDataFrame
from pymovements.events.processing import EventGazeProcessor
from pymovements.gaze import GazeDataFrame
from pymovements.reading_measures.frame import ReadingMeasures

Expand Down Expand Up @@ -713,7 +712,7 @@ def compute_event_properties(
name: str | None
Process only events that match the name. (default: None)
verbose : bool
If ``True``, show progress bar. (default: True)
If ``True``, show more info. (default: True)

Raises
------
Expand All @@ -728,22 +727,9 @@ def compute_event_properties(
Dataset
Returns self, useful for method cascading.
"""
processor = EventGazeProcessor(event_properties)

identifier_columns = [
column
for column in self.fileinfo['gaze'].columns
if column != 'filepath'
]

disable_progressbar = not verbose
for events, gaze in tqdm(zip(self.events, self.gaze), disable=disable_progressbar):
new_properties = processor.process(
events, gaze, identifiers=identifier_columns, name=name,
)
join_on = identifier_columns + ['name', 'onset', 'offset']
events.add_event_properties(new_properties, join_on=join_on)

for gaze in tqdm(self.gaze, disable=disable_progressbar):
gaze.compute_event_properties(event_properties, name=name, verbose=verbose)
return self

def compute_properties(
Expand Down
2 changes: 1 addition & 1 deletion src/pymovements/events/processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def process(
events: EventDataFrame
Event data to process event properties from.
gaze: pm.GazeDataFrame
Gaze data to process event properties from.
Raw gaze data to process event properties from.
identifiers: str | list[str]
Column names to join on events and gaze dataframes.
name: str | None
Expand Down
55 changes: 55 additions & 0 deletions src/pymovements/gaze/gaze_dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from __future__ import annotations

import inspect
import logging
import warnings
from collections.abc import Callable
from copy import deepcopy
Expand All @@ -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.

Expand Down Expand Up @@ -547,6 +552,7 @@
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.

Expand Down Expand Up @@ -883,6 +889,55 @@
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,
) -> pm.EventDataFrame:
senisioi marked this conversation as resolved.
Show resolved Hide resolved
"""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(

Check warning on line 924 in src/pymovements/gaze/gaze_dataframe.py

View check run for this annotation

Codecov / codecov/patch

src/pymovements/gaze/gaze_dataframe.py#L924

Added line #L924 was not covered by tests
'No events available to compute event properties. '
'Consider calling detect before.',
)

if verbose is True:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To log the debug message one would have to set verbose to True and set the logging level to DEBUG.

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

Copy link
Author

@senisioi senisioi Nov 9, 2024

Choose a reason for hiding this comment

The 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;
the message is not very informative either as it shows the same thing over and over again when the method is called from the Dataset

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)
return self

def measure_samples(
self,
method: str | Callable[..., pl.Expr],
Expand Down
Loading