Joss-reviews: [REVIEW]: AxoPy: A Python Library for Implementing Human-Computer Interface Experiments

Created on 21 Jan 2019  ยท  37Comments  ยท  Source: openjournals/joss-reviews

Submitting author: @ixjlyons (Kenneth Lyons)
Repository: https://github.com/axopy/axopy
Version: 0.2.3
Editor: @trallard
Reviewer: @lucask07, @peircej
Archive: 10.5281/zenodo.2562630

Status

status

Status badge code:

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

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) 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

@lucask07 & @peircej, 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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @trallard know.

โœจ Please try and complete your review in the next two weeks โœจ

Review checklist for @lucask07

Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Version: 0.2.3
  • [x] Authorship: Has the submitting author (@ixjlyons) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

Software paper

  • [x] Authors: Does the paper.md file include a list of authors with their affiliations?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @peircej

Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [ ] Version: 0.2.3
  • [x] Authorship: Has the submitting author (@ixjlyons) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

Software paper

  • [x] Authors: Does the paper.md file include a list of authors with their affiliations?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
accepted published recommend-accept review

Most helpful comment

@lucask07, @peircej - many thanks for your reviews and to @trallard for editing this submission โœจ

@ixjlyons - your paper is now accepted into JOSS :zap::rocket::boom:

All 37 comments

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @lucask07, it looks like you're currently assigned as the reviewer for this paper :tada:.

: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
Attempting PDF compilation. Reticulating splines etc...

Some important notes about the revision:

  • We ideally want the review to be conducted within a couple of weeks, but if any of you are struggling for time, please let me know
  • As you go over the checklist: if you encounter any issues or suggest any changes to the package feel free to use this issue as a discussion platform as well as opening issues on the submission repo as needed.
  • If there is anything that is not clear in the reviewers manual or you have any questions about the review process, please feel free to ping me here

Hi @ixjlyons, this package is looking good, very nice work.

Installation via pip (OSX Python 3.7) proceeded without issues:

python -m pip install axopy

However, when importing axopy I hit a snag.

import axopy
ImportError: cannot import name 'Transmitter' from 'axopy.messaging' 

Note that this works:

from axopy.messaging import transmitter   

so I'm suspecting just a capatalization issue?

@lucask07 my apologies. It turns out there were some stale build files lying around on my system so two versions of the messaging submodule ended up in the packages uploaded to PyPI (the old version and the refactored one). I just did a clean build and uploaded version 0.2.2 to PyPI, so you can try again or just upgrade:

python -m pip install --upgrade axopy

@trallard is there a way to bump the version under review to 0.2.2 since the 0.2.1 package is broken? Or is the version number of much consequence for JOSS? This is the only change just to upload a fixed package to PyPI so it shouldn't affect the other review if it's under way.

Thanks @ixjlyons
The import of axopy is working now.

I'm attempting to run the example within the README file. The QT window seems stuck and only reports "Ready". I tried this in an iPython window and without iPython. It would be helpful to know the expected output of this example. I'm running MacOSX.

A screenshot is below:
screen shot 2019-01-26 at 12 40 08 pm

Hey @ixjlyons we can definitely bump the version in JOSS. I'll wait until the review is completed in case the reviewers request any more substantial changes or you make another release in the meantime

@lucask07 yes, that looks like it's working as expected. You should be able to press Enter (with the Qt window focused) and then the screen should show an "oscilloscope" with some random signals updating periodically. Pressing Enter again should then finish the experiment and close the window. One thing to note is running an experiment in an interactive shell isn't something I've intended to support, so it looks like there are a few buggy behaviors when doing so. I foresee this as potentially being an issue for people using an IDE that runs things in an interactive shell like Spyder, so I'll work on finding a solution and track things here.

edit: Oh and the takeaway here at least for now is I need to add a note in the README explaining how the GUI works regarding progression through the tasks.

@ixjlyons
Great, I didn't know to press Enter
Now I see real time updates as an oscilloscope. And it works in the IPython console.

One side note. Python aborted when I requested the power spectrum:

line 683, in _fourierTransform
    y = abs(f[1:len(f)/2])
TypeError: slice indices must be integers or None or have an __index__ method
Abort trap: 6

Ah, that appears to be an upstream issue. There is a way to selectively disable that operation, but I'm currently leaning toward disabling the context menu altogether. It adds a few nice features, but it's probably best if AxoPy users implement their own processing in the form of the pipeline keyword arg in creating the Oscilloscope task.

Hi @ixjlyons,
Thank you for this software it looks great. The documentation is very nicely done. ๐Ÿ˜€
My only suggestion would be to emphasize examples that involve connecting a real-world DAQ.
The pymcc is a useful example, but explicitly knowing things like the needed format of the return data from the read method. I'm thinking about using the Oscilloscope connected to a USB3.0 DAQ system that transfers ~20 MB/s.

So two questions:

  1. What format must the data returned by read be?
  2. What is the maximum data rate that the oscilloscope waveform display can keep up with?

@trallard Hi Tania,
I accept this submission and am finished with my review.

I agree. The paper looks all fine. The software looks sensible, useful and following best practices.
Minor suggestions around docs:

  • I also hit the snag of not realising that I should hit to start simulating the data collection. (I opened the script file and found it mentioned inside the script, but I did this after trying to run). Rather than adding this in the documentation, I would add it to the actual screen "Ready/nHit to start". Lots of people will jump into a demo before reading anything.
  • I'd also include the demos in the package itself (the first place I looked for demos was site-packages and then went online and had to download them separately)
  • Lastly, for projects where the documentation isn't directly at the home page (e.g. hope page is github) it's best to include a link to the readthedocs documentation very clearly at the top of the readme file. When someone arrives at https://github.com/axopy/axopy what steps will they be taking to find the (formatted) docs?

But these are minor things. Useful package, put together nicely (and I love the logo!)

I also accept this submission.

Thank you both @peircej and @lucask07 for your reviews. In a nutshell, I agree with all proposed changes and have made them. Here they are as I've understood:

  • There was a packaging error that caused v0.2.1 to be broken.
  • A context menu provided by an upstream library caused a crash. I disabled the menu, at least until the upstream issue is fixed.
  • It was totally unclear what to do upon running the example in the README. I added instructions to the "ready" screen telling the user what to do to proceed. Thanks @peircej for the much more elegant method of conveying that info :)
  • The DAQ API didn't say anything about the expected format of the data. I added some info about this in the documentation.
  • Links to documentation were fairly hidden in the README. I added a link at the top.
  • The examples were not very easy to find/obtain, given that they weren't packaged and weren't mentioned in the documentation. I can see some benefits to including the examples in the package itself (so it can be run like python -m axopy.examples.example), but for now I've opted to use sphinx-gallery to render the examples in the documentation.

The changes can be seen here: https://github.com/axopy/axopy/compare/181eab2d2a9f203c61b0a82b259538cdf838d890...master


@lucask07 to answer your questions:

I'm thinking about using the Oscilloscope connected to a USB3.0 DAQ system that transfers ~20 MB/s.

So two questions:

What format must the data returned by read be?
What is the maximum data rate that the oscilloscope waveform display can keep up with?

I added a small note about this in the docs, but basically it's up to you. So far in AxoPy, there's an "encouraged" return type: ndarray with shape (n_channels, n_samples). I say encouraged because you could always just implement some transformations as a pipeline block to get it into the format needed by Oscilloscope :). This may become a problem, however, if that can't be done fast enough with the addition of plotting time... AxoPy uses pyqtgraph in the Oscilloscope -- you'd likely have to downsample, but it can plot quite fast (there are some pyqtgraph examples that demonstrate). The process of creating the ndarray may be the problem at that rate. If you try it, let me know.

Hi all thanks for the time spent in this review, it seems that all the issues have already been dealt with and there are just a few steps between acceptance and now.

@peircej can you please make sure to complete the following tasks before completing this?

  • [x] make a new release of the package (unless the latest release is up to date with the recent modifications) and add let us know what is the most up to date version
  • [x] upload the revised software to your DOI-granting data/software repository, and post the DOI here
  • [x] double check the submission paper to ensure it is up-to-date (citations are correct, DOIs are added, etc)

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

Minor: I think the request above was meant for the author @ixjlyons rather than me (to update the version etc).

Ok, here's the DOI for axopy-0.2.3: 10.5281/zenodo.2562630

Thanks once again @trallard, @peircej, and @lucask07 for your time and feedback.

@whedon set 10.5281/zenodo.2562630 as archive

@whedon set 0.2.3 as version

OK. 0.2.3 is the version.

@ixjlyons all looks good to me so I am going to proceed for acceptance

@lucask07 and @peircej thanks a lot for your time and valuable contribution to JOSS as reviewers for this submission ๐Ÿ™Œ๐Ÿป

@arfon: this submission is accepted and ready to be published ๐ŸŽ‰๐Ÿ‘พ

@whedon set 10.5281/zenodo.2562630 as archive

OK. 10.5281/zenodo.2562630 is the archive.

@whedon accept

Attempting dry run of processing paper acceptance...

```Reference check summary:

OK DOIs

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

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

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

@whedon accept deposit=true

Doing it live! Attempting automated processing of paper acceptance...

๐Ÿšจ๐Ÿšจ๐Ÿšจ THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! ๐Ÿšจ๐Ÿšจ๐Ÿšจ

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/497
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01191
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! ๐ŸŽ‰๐ŸŒˆ๐Ÿฆ„๐Ÿ’ƒ๐Ÿ‘ป๐Ÿค˜

    Any issues? notify your editorial technical team...

@lucask07, @peircej - many thanks for your reviews and to @trallard for editing this submission โœจ

@ixjlyons - your paper is now accepted into JOSS :zap::rocket::boom:

@arfon: this submission is accepted and ready to be published ๐ŸŽ‰๐Ÿ‘พ

@trallard - in the future could you please ping @openjournals/joss-eics when a review is complete? That way, the managing editor on duty can process the submission.

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](http://joss.theoj.org/papers/10.21105/joss.01191/status.svg)](https://doi.org/10.21105/joss.01191)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01191">
  <img src="http://joss.theoj.org/papers/10.21105/joss.01191/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.01191/status.svg
   :target: https://doi.org/10.21105/joss.01191

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

Was this page helpful?
0 / 5 - 0 ratings