🍄 events moved to gaze df

[senisioi]

Description

• issue 871: Add GazeDataFrame.compute_event_properties() #871

Fixes #

Implemented changes

Insert a description of the changes implemented in the pull request.

• [] added the functionality to compute event properties at the gaze datafram level
• [] raised an error if the gaze object did not run detect events before
• [] added logging messages for verbose parameter to make sense

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change is or requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration

• Test A
• Test B

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• I have checked my code and corrected any misspellings

[codecov]

Codecov Report

Attention: Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 99.79%. Comparing base (502dc44) to head (8982b9d).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
src/pymovements/gaze/gaze_dataframe.py	76.92%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##              main     #885      +/-   ##
===========================================
- Coverage   100.00%   99.79%   -0.21%     
===========================================
  Files           72       74       +2     
  Lines         3321     3385      +64     
  Branches       594      596       +2     
===========================================
+ Hits          3321     3378      +57     
- Misses           0        5       +5     
- Partials         0        2       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dkrako]

Thank you so much! This PR is already in great shape!

There are some small quirks left to work out, especially regarding the logging/verbose parts.

[dkrako]

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.

[senisioi]

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

[dkrako]

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

[senisioi]

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?