Repository navigation
Conversation
Add getTouchActors(), getTouchActorAois() and getTouchActorRespondents() to list a stimulus's touch actors, pair them with their interactive AOIs and contact in/out data, and filter respondents that have contact data for a stimulus. Add getAoiRespondentTouchData() and getAoiRespondentTouchMetrics() to retrieve the contact in/out signal, active intervals, and metrics for a (touch actor, AOI)/respondent combination. Add supporting private helpers (privateGetTouchActorDetails, privateDecodeHandType) and endpoint builders (getTouchActorsUrl, getTouchActorDetailsUrl S3 methods for imAOI/imStimulus). Add tests and fixtures covering the new endpoints and helpers.
Contributor
Test Results 1 files 68 suites 13s ⏱️ Results for commit b1d8edb. ♻️ This comment has been updated with latest results. |
…n for loop using <- — fixes the assignment_linter error. Removed one space from the NA_real_)] continuation to match the expected 63-space hanging indent — fixes the indentation_linter warning.
agrappe
reviewed
Aug 13, 2026
agrappe
reviewed
Aug 13, 2026
agrappe
reviewed
Aug 14, 2026
- agrappe: "Do you plan to handle these one? We will need to create export
files, this information will be needed." -> Added
getAoiRespondentTouchData()/getAoiRespondentTouchMetrics() to expose
contact in/out data and metrics beyond the UI.
- agrappe: accepting both `stimulus` and an optional `touchActor` allowed
contradictory input (e.g. a touch actor from a different stimulus) ->
getTouchActorAois() now takes a single `imObject` argument, dispatching
on whether it's an imStimulus or an imTouchActor.
- agrappe: "when imObject is an imTouchActor, retrieve its stimulus via
getStimulus(study, imObject$stimulusId)" -> done; the touch actor's own
stimulusId now drives the AOI lookup, with the original touch actor kept
aside only to filter pairs down to it afterwards.
- agrappe: call privateAoiFiltering() directly instead of getAois(), to
avoid manually resetting the class back to data.table before the merge
-> done; also removed the now-unneeded class(aois) <- c("data.table",
"data.frame") workaround.
- agrappe: use unnest(pairs, cols = "aoiId") instead of the manual
data.table unlist/by -> done.
- agrappe: introduce an imTouchAOI/imTouchAOIList subclass of imAOI(List)
so a plain gaze AOI can't be passed to touch-specific functions, and
replace getTouchActorDetailsUrl.imAOI with getTouchActorDetailsUrl.imTouchAOI
-> done; getTouchActorAois() now returns imTouchAOI(List), and all
touch-specific functions assertClass() on it instead of imAOI.
- agrappe: introducing imTouchAOI should let getRespondents(study, AOI =
touchAoi) use contact details instead of the gaze in/out method; delete
getTouchActorRespondents() and add a dedicated
privateRespondentFiltering.imTouchAOI S3 method -> done. Without this,
getRespondents(study, AOI = touchAoi) silently fell through to the gaze
method (imTouchAOI passes assertClass(AOI, "imAOI")) and returned an
incorrect, misleadingly empty result on a touch-only study.
privateRespondentFiltering.imTouchAOI() resolves the AOI's stimulus,
filters to respondents exposed to it, then keeps only those whose contact
details contain a matching (touchActorId, aoiId) pair.
getTouchActorRespondents() is removed, superseded by this dispatch.
- agrappe: assertClass(touchAoi, "imTouchAOI", ...) should be used in every
touch-specific function -> done throughout.
- Zurisen: replying on keeping getTouchActorAois() flexible to accept either
a stimulus or a touch actor -> matches the imObject dispatch implemented
above.
- agrappe: "Do you plan to handle these one? We will need to create export
files, this information will be needed." -> Added
getAoiRespondentTouchData()/getAoiRespondentTouchMetrics() to expose
contact in/out data and metrics beyond the UI.
- agrappe: accepting both `stimulus` and an optional `touchActor` allowed
contradictory input (e.g. a touch actor from a different stimulus) ->
getTouchActorAois() now takes a single `imObject` argument, dispatching
on whether it's an imStimulus or an imTouchActor.
- agrappe: "when imObject is an imTouchActor, retrieve its stimulus via
getStimulus(study, imObject$stimulusId)" -> done; the touch actor's own
stimulusId now drives the AOI lookup, with the original touch actor kept
aside only to filter pairs down to it afterwards.
- agrappe: call privateAoiFiltering() directly instead of getAois(), to
avoid manually resetting the class back to data.table before the merge
-> done; also removed the now-unneeded class(aois) <- c("data.table",
"data.frame") workaround.
- agrappe: use unnest(pairs, cols = "aoiId") instead of the manual
data.table unlist/by -> done.
- agrappe: introduce an imTouchAOI/imTouchAOIList subclass of imAOI(List)
so a plain gaze AOI can't be passed to touch-specific functions, and
replace getTouchActorDetailsUrl.imAOI with getTouchActorDetailsUrl.imTouchAOI
-> done; getTouchActorAois() now returns imTouchAOI(List), and all
touch-specific functions assertClass() on it instead of imAOI.
- agrappe: introducing imTouchAOI should let getRespondents(study, AOI =
touchAoi) use contact details instead of the gaze in/out method; delete
getTouchActorRespondents() and add a dedicated
privateRespondentFiltering.imTouchAOI S3 method -> done. Without this,
getRespondents(study, AOI = touchAoi) silently fell through to the gaze
method (imTouchAOI passes assertClass(AOI, "imAOI")) and returned an
incorrect, misleadingly empty result on a touch-only study.
privateRespondentFiltering.imTouchAOI() resolves the AOI's stimulus,
filters to respondents exposed to it, then keeps only those whose contact
details contain a matching (touchActorId, aoiId) pair.
getTouchActorRespondents() is removed, superseded by this dispatch.
- agrappe: assertClass(touchAoi, "imTouchAOI", ...) should be used in every
touch-specific function -> done throughout.
- Zurisen: replying on keeping getTouchActorAois() flexible to accept either
a stimulus or a touch actor -> matches the imObject dispatch implemented
above.
getTouchActorAois() joined every (touch actor, AOI) pair against privateAoiFiltering(), which only ever reaches the Stimulus/Scene camera's AOI set. A touch actor whose interactive AOI lives on another camera (e.g. Environment) matched nothing in that join, so its name/type/group/area came back NA even though contact detection and metrics were unaffected. Add privateGetTouchActorAoiDefinitions(), backed by the desktop's new GET /touchactors/<study>/stimuli/<stim>/aois endpoint, which reports AOI definitions across every touch-capable camera. getTouchActorAois() now joins against that instead, resolving names/types/areas regardless of camera - AOI ids are unique per camera's own model, so the join key doesn't need to change. This is shared by the UI grid pipeline (mainContactAggregate.R, processContactRespondentData.R) as well as the export script, so the fix applies to both without touching either. Updated the existing regression test that had asserted the old NA behaviour as correct, added a passing-case test for a non-Scene AOI, and a fallback test for when no AOI definitions are available at all. Verified: full package test suite passes (0 failures). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
agrappe
reviewed
Aug 24, 2026
agrappe
reviewed
Aug 24, 2026
agrappe
reviewed
Aug 24, 2026
agrappe
reviewed
Aug 24, 2026
agrappe
reviewed
Aug 24, 2026
…hain getTouchActorAois() previously required a respondent up front, unlike getAois() and friends, which resolve respondents via the API instead. This mirrors that pattern: respondent is now optional across getTouchActorAois(), privateGetTouchActorDetails(), and every getTouchActorDetailsUrl.* method, so callers can list touch actor AOI definitions first and use getRespondents(study, AOI = touchAoi) to discover who has contact data for a given (touch actor, AOI) pair - resolving the circular dependency where getting a touchAoi previously needed an already-known respondent. Also addresses the remaining PR #37 review comments: - getTouchActorAois() reuses an imTouchActor argument directly instead of re-fetching and filtering every touch actor on the stimulus. - privateGetTouchActorDetails() is now called with the actor-scoped imObject rather than the whole stimulus, so contact detection only runs for the requested touch actor instead of every one on it - via a new getTouchActorDetailsUrl.imTouchActor method.
agrappe
reviewed
Aug 26, 2026
agrappe
reviewed
Aug 26, 2026
agrappe
reviewed
Aug 26, 2026
agrappe
reviewed
Aug 26, 2026
- Scope privateGetTouchActorDetails() down to the requested (touch actor, AOI) pair when given an imTouchAOI, instead of returning every interactive AOI of that touch actor. - Make privateUploadAoiMetrics.imRespondent() dispatch explicitly to privateGetTouchActorDetails() for touch AOIs, rather than relying on incidental shape overlap with privateGetAoiDetails(). - Add the missing `study` argument validation to getTouchActorAois(). - Document imTouchAOI as an accepted imObject in privateGetTouchActorDetails()'s roxygen, matching the other two accepted classes, and mention it as an accepted AOI input in uploadAoiMetrics()/privateUploadAoiMetrics().
…e utility functions
agrappe
reviewed
Sep 11, 2026
…chActorAois pattern.
…rics # Conflicts: # imotionsApi/NAMESPACE # imotionsApi/R/imotionsApi.R # imotionsApi/tests/testthat/test-uploadAOIMetrics.R
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refer to PR: Add AOI contact metrics pipeline (touch actors)
Add getTouchActors(), getTouchActorAois() and getTouchActorRespondents() to list a stimulus's touch actors, pair them with their interactive AOIs and contact in/out data, and filter respondents that have contact data for a stimulus.
Add getAoiRespondentTouchData() and getAoiRespondentTouchMetrics() to retrieve the contact in/out signal, active intervals, and metrics for a (touch actor, AOI)/respondent combination.
Add supporting private helpers (privateGetTouchActorDetails, privateDecodeHandType) and endpoint builders (getTouchActorsUrl, getTouchActorDetailsUrl S3 methods for imAOI/imStimulus).
Add tests and fixtures covering the new endpoints and helpers.