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.

Sign up for GitHub

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

Jump to bottom

feat: split_gaze_data by column values #859

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

feat: split_gaze_data by column values #859

wants to merge 32 commits into from
+125 −2

Conversation

SiQube
Copy link
Member

@SiQube SiQube commented Oct 23, 2024

the newly added gaze files in the copco dataset, are on a per-subject file basis. unfortunately many trials are within one file -- this (a bit hacky) feature gives a user the possibility to split data into trial-level GazeDataFrames.

@github-actions github-actions bot added the enhancement New feature or request label Oct 23, 2024
@codecov Codecov
Copy link

codecov bot commented Oct 23, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (29b806d) to head (8994bb1).

Additional details and impacted files 
@@            Coverage Diff            @@
##              main      #859   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           74        74           
  Lines         3419      3432   +13     
  Branches       613       614    +1     
=========================================
+ Hits          3419      3432   +13

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

@SiQube SiQube force-pushed the split-gaze-files-into-trial-dataframes branch from c557a78 to 16b7ae7 Compare October 23, 2024 06:46
@SiQube SiQube force-pushed the split-gaze-files-into-trial-dataframes branch from 16b7ae7 to 1c6c769 Compare October 23, 2024 10:54
dkrako
dkrako requested changes Oct 24, 2024
View reviewed changes
src/pymovements/dataset/dataset.py Outdated Show resolved Hide resolved
dkrako
dkrako requested changes Nov 7, 2024
View reviewed changes
src/pymovements/dataset/dataset.py Outdated Show resolved Hide resolved
src/pymovements/dataset/dataset.py Outdated Show resolved Hide resolved
dkrako and others added 19 commits November 17, 2024 19:50
@dkrako @SiQube
docs: Add missing modules to documentation (#866)
976695b 
Some modules where missing from the html documentation.
This PR adds these.
@SiQube
hotfix: check whether public dataset has gaze files (#872)
953ade3
@dkrako @SiQube
docs: correctly add EyeTracker class to gaze module (#876)
b842bdb 
* docs: Add missing EyeTracker class to html docs

* the eye tracker class was not correctly integrated somehow
@SiQube
feat: add support for .ias files in stimulus.text.from_file() (#858)
cc1bae1
@SiQube
dataset: beijing sentence corpus (#857)
5417804
@SiQube
dataset: add InteRead dataset (#862)
92b49a7
@SiQube
fix: copy event resource files instead of moving them to events direc…
f0b69a9 
…tory (#863)
@SiQube
hotfix: CopCo dataset precomputed eventsloading (#873)
e6a9ced
@dkrako @SiQube
ci: ignore too-many-public-methods (#882)
1b8c4bd
@pre-commit-ci @SiQube
ci: pre-commit autoupdate (#889)
69ef837 
updates:
- [github.com/asottile/pyupgrade: v3.18.0 → v3.19.0](asottile/pyupgrade@v3.18.0...v3.19.0)
- [github.com/pre-commit/mirrors-mypy: v1.12.1 → v1.13.0](pre-commit/mirrors-mypy@v1.12.1...v1.13.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@pre-commit-ci @SiQube
ci: pre-commit autoupdate (#890)
cfbce95 
updates:
- [github.com/kynan/nbstripout: 0.7.1 → 0.8.0](kynan/nbstripout@0.7.1...0.8.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@SiQube
build: add support for python 3.13 (#845)
47e734d
@dependabot @SiQube
build: update nbsphinx requirement from <0.9.5,>=0.8.8 to >=0.8.8,<0.…
166b076 
…9.6 (#777)

Updates the requirements on [nbsphinx](https://github.com/spatialaudio/nbsphinx) to permit the latest version.
- [Release notes](https://github.com/spatialaudio/nbsphinx/releases)
- [Changelog](https://github.com/spatialaudio/nbsphinx/blob/master/NEWS.rst)
- [Commits](spatialaudio/nbsphinx@0.8.8...0.9.5)

---
updated-dependencies:
- dependency-name: nbsphinx
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
@pre-commit-ci @SiQube
ci: pre-commit autoupdate (#896)
495e5d9 
updates:
- [github.com/nbQA-dev/nbQA: 1.8.7 → 1.9.1](nbQA-dev/nbQA@1.8.7...1.9.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@dependabot @SiQube
build: update setuptools-git-versioning requirement from <2 to <3 (#895)
b691e6d 
Updates the requirements on [setuptools-git-versioning](https://github.com/dolfinus/setuptools-git-versioning) to permit the latest version.
- [Release notes](https://github.com/dolfinus/setuptools-git-versioning/releases)
- [Changelog](https://github.com/dolfinus/setuptools-git-versioning/blob/master/CHANGELOG.rst)
- [Commits](dolfinus/setuptools-git-versioning@v0.0.1...v2.0.0)

---
updated-dependencies:
- dependency-name: setuptools-git-versioning
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@SiQube @aarbeikop
hotfix: download link fakenewsperception dataset (#897)
88113c8 
Co-authored-by: aarbeikop <[email protected]>
@saeub @faizygithub @SiQube
feat: Store metadata from ASC in experiment metadata (#884)
21fd0d2 
* updated io file

* updated test file

* Add tests for metadata parsing from ASC file

* Squashed commit of the following:

commit 14d047c
Author: Faizan Ansari <[email protected]>
Date:   Thu Oct 24 22:02:30 2024 +0200

    Remove files from remote directory

commit aa78078
Author: Faizan Ansari <[email protected]>
Date:   Thu Oct 24 21:53:35 2024 +0200

    updated code

commit cae54cc
Author: Faizan Ansari <[email protected]>
Date:   Thu Oct 24 15:40:16 2024 +0200

    changes in io.py file

* Fix formatting

* Fix indentation

* Fix circular imports

* 2 test passed

* Fix attribute name

* Refactor metadata checks, add tests

* Fix f-strings

* Fix tests

* Address comments

* Improve test coverage

* Add comment about screen resolution

* Fix metadata conflict check

* Fix test coverage

* Fix type hint

* Trigger codecov

* rebase me

* Upgrade codecov action

* Revert codecov action upgrade

---------

Co-authored-by: Faizan Ansari <[email protected]>
Co-authored-by: SiQube <[email protected]>
@SiQube
move split method to gaze dataframe
0856658
@SiQube
Merge branch 'main' into split-gaze-files-into-trial-dataframes
4751e41
dkrako
dkrako requested changes Dec 8, 2024
View reviewed changes
Copy link
Contributor

@dkrako dkrako left a comment

Choose a reason for hiding this comment

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

Thank you so much for your work!

I think there are still some rough edges we need to iron out. There's probably some misunderstanding about how the column specifiers are treated during init.

Adding proper tests of GazeDataFrame.split() will probably resolve these issues.

src/pymovements/gaze/gaze_dataframe.py Outdated Show resolved Hide resolved
src/pymovements/gaze/gaze_dataframe.py Outdated Show resolved Hide resolved
src/pymovements/gaze/gaze_dataframe.py Outdated Show resolved Hide resolved
src/pymovements/gaze/gaze_dataframe.py Outdated Show resolved Hide resolved
src/pymovements/gaze/gaze_dataframe.py Outdated Show resolved Hide resolved
src/pymovements/gaze/gaze_dataframe.py Show resolved Hide resolved
src/pymovements/gaze/gaze_dataframe.py Outdated Show resolved Hide resolved
src/pymovements/gaze/gaze_dataframe.py Outdated Show resolved Hide resolved
tests/unit/dataset/dataset_test.py Outdated Show resolved Hide resolved
src/pymovements/dataset/dataset.py Outdated Show resolved Hide resolved
@pre-commit-ci @SiQube
ci: pre-commit autoupdate (#899)
b47ad31 
updates:
- [github.com/pappasam/toml-sort: v0.23.1 → v0.24.1](pappasam/toml-sort@v0.23.1...v0.24.1)
- [github.com/kynan/nbstripout: 0.8.0 → 0.8.1](kynan/nbstripout@0.8.0...0.8.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
pre-commit-ci bot and others added 11 commits December 28, 2024 15:48
@pre-commit-ci @SiQube
ci: pre-commit autoupdate (#900)
5f5525a 
updates:
- [github.com/pappasam/toml-sort: v0.24.1 → v0.24.2](pappasam/toml-sort@v0.24.1...v0.24.2)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@saeub @SiQube
Add trial_columns argument in from_asc() (#898)
c30bd9e
@pre-commit-ci @SiQube
ci: pre-commit autoupdate (#902)
7a25297 
updates:
- [github.com/PyCQA/pylint: v3.3.1 → v3.3.2](pylint-dev/pylint@v3.3.1...v3.3.2)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@SiQube
docs: add CITATION.cff (#901)
96141d5
@pre-commit-ci @SiQube
ci: pre-commit autoupdate (#904)
5bf55f1 
updates:
- [github.com/jsh9/pydoclint: 0.5.9 → 0.5.10](jsh9/pydoclint@0.5.9...0.5.10)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@dkrako @SiQube
ci: add dataset section to release drafter (#903)
e4b3e8f
@SiQube
move split method to gaze dataframe
eb8aee5
@SiQube
add tests for number of split files
ecd6b5c 
only one trial id per gaze dataframe
@SiQube
Merge branch 'main' into split-gaze-files-into-trial-dataframes
7229cd9
@SiQube
Merge branch 'main' into split-gaze-files-into-trial-dataframes
52902af
@SiQube
Merge branch 'main' into split-gaze-files-into-trial-dataframes
8994bb1
dkrako
dkrako requested changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@dkrako dkrako left a comment

Choose a reason for hiding this comment

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

Great! Thank you so much for taking care of all the PRs!

I'd just like to change the type to be more open to sequence types other than just lists.

src/pymovements/gaze/gaze_dataframe.py Show resolved Hide resolved
src/pymovements/dataset/dataset.py
@@ -231,6 +231,30 @@ def load_precomputed_reading_measures(self) -> None:
self.paths,
)

def split_gaze_data(
self,
by: list[str] | str,
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use Sequence here from collections, this way it's more in line with the polars signature: https://docs.pola.rs/api/python/stable/reference/dataframe/api/polars.DataFrame.partition_by.html

src/pymovements/gaze/gaze_dataframe.py
@@ -307,6 +308,33 @@ def apply(
else:
raise ValueError(f"unsupported method '{function}'")

def split(self, by: list[str] | str) -> list[GazeDataFrame]:
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use Sequence[str]

@dkrako dkrako changed the title feat: split_gaze_data into trial feat: split_gaze_data by column values Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkrako dkrako dkrako requested changes

@prassepaul prassepaul Awaiting requested review from prassepaul prassepaul is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SiQube @dkrako @saeub