feat: split TextStimulus by column values
#879
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #879 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 74 74
Lines 3419 3422 +3
Branches 613 613
=========================================
+ Hits 3419 3422 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I'm not sure if the Textstimulus.aois attribute should change types.
Before calling split_aois_by() the type is pl.DataFrame afterwards it's a list of dataframes. It would be nice to have just one type for the aois field.
Also, when you just split the data, you won't know the splitting criteria anymore, i.e. you won't know which dataframe is which belonging to which page.
Can you include a short code example where and how you expect the method to be used? It is probably part of a greater plan.
…into aoi_events x
…into aoi_events x
…into aoi_events x
…into aoi_events x
There was a problem hiding this comment.
Thank you @izaskr for your work!
Unfortunately I have missed a crucial line during my review two weeks ago as the split() method doesn't return list[TextStimulus].
I have posted a small code snippet how to change the logic accordingly, shouldn't be much work.
This change would also need an additional test: please check that all fields (e.g. aoi_column, width_column, etc.) in the returned TextStimulus splits are equal to the original TextStimulus.
dkrako
changed the title
TextStimulus by column values
Description
The change introduces a splitting function for the aoi dataframe in the TextStimulus class. This allows for the aoi file to be split according to some criteria, e.g. pages.
Implemented changes
Insert a description of the changes implemented in the pull request.
Type of change
How Has This Been Tested?
test_text_stimulus_splittingchecks if the number of resulting dataframes equals the expected length.Checklist: