search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
720 stars 38 forks source link

[REVIEW]: pypillometry: A Python package for pupillometric analyses #2348

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @ihrke (Matthias Mittner) Repository: https://github.com/ihrke/pypillometry Version: v1.0.0 Editor: @oliviaguest Reviewers: @samhforbes, @szorowi1, @tjmahr Archive: 10.5281/zenodo.3925528

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/3b06f4f3d5b703fd99c7e622b7edebe4"><img src="https://joss.theoj.org/papers/3b06f4f3d5b703fd99c7e622b7edebe4/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/3b06f4f3d5b703fd99c7e622b7edebe4/status.svg)](https://joss.theoj.org/papers/3b06f4f3d5b703fd99c7e622b7edebe4)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@samhforbes & @szorowi1, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @oliviaguest know.

✨ Please try and complete your review in the next six weeks ✨

Review checklist for @samhforbes

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @szorowi1

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @tjmahr

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @samhforbes, @szorowi1 it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.5334/joc.18 is OK
- 10.1126/science.132.3423.349 is OK
- 10.3758/s13428-020-01374-8 is OK
- 10.1146/annurev.neuro.28.061604.135709 is OK
- 10.3758/cabn.10.2.252 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

oliviaguest commented 4 years ago

Hi all! 👋 Thank you so much, @samhforbes, @szorowi1, @tjmahr for accepting to review this software package and paper by @ihrke! 😊

Any questions, feedback on the paper, etc., please post here. Any very code-specific questions, suggestions, etc., please use the issues in the code repo and link to them from this thread so we can all keep track of them. 🌸

Thank you again, Sams and Tristan!

oliviaguest commented 4 years ago

@whedon add @tjmahr as reviewer

whedon commented 4 years ago

OK, @tjmahr is now a reviewer

oliviaguest commented 4 years ago

Can @samhforbes, @szorowi1, and @tjmahr give me a rough ETA on their reviews please? ☺️

samhforbes commented 4 years ago

Installation instructions: Is there a clearly-stated list of dependencies? Link to more in depth installation instructions for PySTan: https://github.com/ihrke/pypillometry/issues/4

Example usage and functionality documentation ideally would have documentation on modelling data. https://github.com/ihrke/pypillometry/issues/1

Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support https://github.com/ihrke/pypillometry/issues/2

References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax? DOIs need to be included for a number of papers. See https://github.com/ihrke/pypillometry/issues/3

ihrke commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

PDF failed to compile for issue #2348 with the following error:

Can't find any papers to compile :-(

oliviaguest commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

oliviaguest commented 4 years ago

@whedon add @tjmahr as reviewer

whedon commented 4 years ago

OK, @tjmahr is now a reviewer

oliviaguest commented 4 years ago

@whedon re-invite @tjmahr as reviewer

whedon commented 4 years ago

The reviewer already has a pending invite.

@tjmahr please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

szorowi1 commented 4 years ago

Hi all, had the pleasure of trying out pypillometry this week. I was successfully able to install pypillometry and then load in my own data, perform preprocessing (blink detection + correction, filtering, rescaling) with ease, and compute/plot ERPDs. The examples were more than enough help to adapt the code to my data, and the API was detailed and easy to read. Overall, the package is very impressive, especially the natural chaining/logging of preprocessing steps. Really cool stuff, @ihrke!

I've made a couple of minor suggestions about installation details here, https://github.com/ihrke/pypillometry/issues/5. I also agree with @samhforbes that more documentation about the modeling functions would be helpful, as is currently being discussed here, https://github.com/ihrke/pypillometry/issues/1.

ihrke commented 4 years ago

Thanks @samhforbes and @szorowi1 for nice comments and suggestions! I have tried to address them in the issues over at ihrke/pypillometry and added the propose-close label. If you are happy with the changes, can you close the respective issues? The remaining issues are

Thanks for a great process so far and looking forward to further comments!

oliviaguest commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

samhforbes commented 4 years ago

@ihrke I'm satisfied that you've answered my queries, and I've ticked those remaining boxes. Well done on what I feel is a really important package.

oliviaguest commented 4 years ago

@samhforbes thank you so much! I might ask you all to check the very final version one last time when all reviewers have given comments and @ihrke has made all edits (e.g., in the rare case there are any divergences of opinion or feedback needed on anything), but for now feel free to go off and do other stuff, Sam H.F.! 👏

tjmahr commented 4 years ago

Thank you for this opportunity to review this package. I have written a few lab-specific in-house R packages for managing eyetracking data, so I applaud the effort to provide a general-purpose tool for working with eyetracking data.

I have two disclaimers: I have not formally analyzed pupil data beyond the data-cleaning stage and I seldom use Python.

The documentation is comprehensive and generously provides several notebooks with binder links. The installation instructions helped me use virtual environments for the first time ever. Functions that provide fake data also help learners play with the package.

I have Python 3.6.4 via Anaconda on Windows 10. Installation via cloning from Github and installing dependencies with pip 18.1 went smoothly. I tested the package on some 60 Hz Tobii data I had from an eyetracking task with preschoolers. I created csvs of gaze measurements and event onsets in R. Then in Python I was able to create a plot of pupil diameter over time with trial events annotated. It was cool to move from my own csvs to a nice default plot in a few steps.

I was unable to use velocity based blink detection (it raised an error), so I looked up the API reference for blink_detect() and tried d.blinks_detect(strategies = ["zero"]). This worked, but it treated all of the intertrial intervals (during which the eyetracker is not running) in the dataset as blinks (below). Fair enough.

image

After downsampling to 5 Hz, I noticed that these same blinks were changed to one spurious blink.

image

At this point, I gave up on trying to use blink detection or interpolation on my data because it was far too noisy and from a task that was never meant to analyze pupil size. That said, the availability of diagnostic plotting, as in d.blinks_interp_mahot(plot=True, plot_dim=(7,4)), is a user-friendly addition.

I have to ask: Is there a minimum sampling frequency required for blink analysis? Does the package make any assumptions about the capability of the eyetracker? How should missing eyetracking data be coded in csv file?

The pipeline based processing is a terrific design. By storing the data in a PupilData object, the data can keep track of the changes made to it. And because the history is itself data, it can be applied to other objects with the apply_history(). This set of features is awesome for reproducibility and usability.

In eyetracking, we often want to see how gaze measures change in response to some event and adjust times around that event. This package's ERPD (event-related pupil dilation) nicely handles this process by allowing users to select an event and time window before and after this event. It also provides automatic plotting of aggregating gaze measurements. I was not able to get it to work on my noisy Tobii data, but I reproduced the plots in the notebook using the supplied data.

I ran through the phasic and tonic components to confirm that I was able to get pystan to do the modeling.

I would recommend this package to any person analyzing pupil data. The history feature is a really smart way to let users prototype a workflow on a single dataset and then quickly apply it to several datasets. My only concern is that it was not clear what requirements are placed on the data.

szorowi1 commented 4 years ago

Just wanted to say that with the closing of https://github.com/ihrke/pypillometry/issues/1, I'm totally satisfied!

Again, really nice work @ihrke! Will absolutely be recommending this package to the folks collecting pupillometry data in my center

ihrke commented 4 years ago

Thanks @tjmahr and @szorowi1 for nice and thorough reviews!

Re your experiences with Tobii data, @tjmahr: the question about requirements for data-quality is a good one and one that is hard to answer in general. With the "spotty" data in your figures, many of the blink-features that are based on velocity profiles and detecting points surrounding the blinks are indeed bound to fail. I also see a couple of non-blink artifacts in your dataset which will seriously lead astray the blink-detection/interpolation. Generalizing from these plots to the rest of the data, to make any valid inferences about PD in the presence of so many missing data may be doubtful. One strategy would be to try and interpolate consistent blinks (i.e., with visible pre-post transients) and then go for a more aggressive interpolation-strategy (i.e., interpolate across all missing data) for the remaining time-series.

pypillometry was developed using mainly data from three eyetrackers in our lab (two different Eyelinks and a SMI) which is collected during stationary laboratory tasks typically using chin-rests and under constant lighting conditions. Hence, data-quality was likely high and pypillometry's functions will be necessarily tailored and biased towards this data. I hope that with time, as more people and labs start using the package, we will be able to get experience with more diverse datasets and match functionality of the package. Perhaps you would like to upload an example data-file and some details about the problems in an issue over at https://github.com/ihrke/pypillometry? I'll leave the decision about whether this is critical for acceptance of the JOSS-paper or not to you.

tjmahr commented 4 years ago

Thanks for the response. (I forgot that some eye trackers have chin rests. I was using data from a 4 year old with a free field eye tracker.) I don’t think it’s critical to address for publication.

I’m satisfied. Ship it. 👁

oliviaguest commented 4 years ago

I’m satisfied. Ship it. 👁

@tjmahr LMFAO! 👏

Thank you so much, all! @szorowi1 @samhforbes @tjmahr — great work! 💯

oliviaguest commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

oliviaguest commented 4 years ago

@ihrke is that looking good to you? If you need to make any changes, please do and compile the PDF again (as I did above). ☺️

ihrke commented 4 years ago

This looks great to me @oliviaguest! Thanks again @samhforbes, @tjmahr and @szorowi1 for a constructive round of reviews!

oliviaguest commented 4 years ago

Oh, nice, we can get this published then! @ihrke can you create an archive of the code (on Zenodo, figshare, or other) and post the archive DOI here, please?

ihrke commented 4 years ago

sure thing! https://doi.org/10.5281/zenodo.3925528

oliviaguest commented 4 years ago

@whedon set 10.5281/zenodo.3925528 as archive

whedon commented 4 years ago

OK. 10.5281/zenodo.3925528 is the archive.

oliviaguest commented 4 years ago

@whedon check references

whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.5334/joc.18 is OK
- 10.1177/1745691611427305 is OK
- 10.1126/science.132.3423.349 is OK
- 10.3758/s13428-020-01374-8 is OK
- 10.21105/joss.02285 is OK
- 10.1146/annurev.neuro.28.061604.135709 is OK
- 10.1016/j.neuron.2015.11.028 is OK
- 10.1016/j.tics.2016.06.004 is OK
- 10.3758/cabn.10.2.252 is OK
- 10.3233/978-1-61499-649-1-87 is OK

MISSING DOIs

- None

INVALID DOIs

- None
oliviaguest commented 4 years ago

@whedon accept

oliviaguest commented 4 years ago

@openjournals/joss-eics please accept this! ✨

oliviaguest commented 4 years ago

@whedon accept

whedon commented 4 years ago 
Attempting dry run of processing paper acceptance...
whedon commented 4 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/1529

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/1529, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
oliviaguest commented 4 years ago

@ihrke dunno why @whedon ignored me the first time — check final proof please and an EiC will pop by soon and do the deal fully. 🥳

ihrke commented 4 years ago

this looks nice, all links in the final PDF are functioning 👍

danielskatz commented 4 years ago

@ihrke - can you edit the metadata in the zenodo archive so that the title matches the title of the paper?

danielskatz commented 4 years ago

Once you do so, this appears ready to publish

ihrke commented 4 years ago

Thanks @danielskatz, I fixed it.

danielskatz commented 4 years ago

@whedon accept deposit=true

whedon commented 4 years ago 
Doing it live! Attempting automated processing of paper acceptance...
whedon commented 4 years ago

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦