search

openjournals / joss-reviews

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

[REVIEW]: SleepPy: A python package for sleep analysis from accelerometer data #1663

Closed whedon closed 4 years ago

whedon commented 5 years ago

Submitting author: @elyiorgos (Yiorgos Christakis) Repository: https://github.com/elyiorgos/sleeppy Version: 0.2.21 Editor: @lheagy Reviewers: @eskofier, @ikarosilva, @nhejazi Archive: 10.5281/zenodo.3572581

Status

status

Status badge code:

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

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

@eskofier & @ikarosilva, 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 @lheagy know.

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

Review checklist for @eskofier

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @ikarosilva

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @nhejazi

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 5 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @eskofier, @ikarosilva it looks like you're currently assigned to review 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

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

@whedon generate pdf
whedon commented 5 years ago 
Attempting PDF compilation. Reticulating splines etc...
whedon commented 5 years ago

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

lheagy commented 5 years ago

Thank you @eskofier and @ikarosilva for being willing to review! Above there is a checklist to help guide your review. Please feel free to open issues on the target repository with comments or suggestions for improvement in the submission and link them here by mentioning openjournals/joss-reviews#1663 in the issue text.

If you are able to complete your review in the next two weeks, that would be much appreciated. Please let me know if you have any questions along the way. Thanks again!

Kevin-Mattheus-Moerman commented 5 years ago

@eskofier, @ikarosilva thanks again for your help with this review. Can you please give us an indication as to when you can work on/finalize this review? Thanks.

eskofier commented 5 years ago

Hi Kevin,

What exactly are we supposed to do? I have not seen any review instructions or the likes?

Best regards, Bjoern

-sent from mobile office-

Am 11.09.2019 um 06:23 schrieb Kevin Mattheus Moerman notifications@github.com:

@eskofier, @ikarosilva thanks again for your help with this review. Can you please give us an indication as to when you can work on/finalize this review? Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Kevin-Mattheus-Moerman commented 5 years ago

@eskofier thanks for your question. Our documentation site contains our review guidelines. It also describes the checklist items (https://joss.readthedocs.io/en/latest/review_checklist.html) which appear at the top of this issue. The checklist is meant to guide you through the review process. In essence we ask you to install/set-up and test the software based on whatever documentation the authors have provided. Next you can evaluate the software functionality. We also ask you to review the very short paper. If you want to leave comments to the authors you can leave them here. For long descriptions/larger software related issue I recommend opening issues on the dedicated project software repository.

Let us know if you have any other questions.

@lheagy is the handling editor for this issue and she'll be able to help answer any further questions you might have during review.

Thanks again for your help!

lheagy commented 5 years ago

Thanks for the overview @Kevin-Mattheus-Moerman!

ikarosilva commented 5 years ago

Hi @lheagy,

Posting here per per our discussion on conflict of interest waiver. I have worked with Dr. Patel for several months and a year on a team at a start up in 2015. I hold him at a very high regards and as a trusted colleague. We also have a few publications in press. While I don't believe our relationship should affect my ability to review this work, I would like to disclose this information in spirit of full transparency.

lheagy commented 5 years ago

Thanks for your disclosure @ikarosilva. You are welcome to contribute to the review, but please leave the check-box for the conflict of interest unckecked. I will try to find one more reviewer to contribute to the review.

lheagy commented 5 years ago

:wave: @eskofier, when do you anticipate that you will have time to take a look at this review? Thanks!

eskofier commented 5 years ago

I am on it!

Best regards, Bjoern

-sent from mobile office-

Am 23.09.2019 um 17:08 schrieb Lindsey Heagy notifications@github.com:

👋 @eskofier, when do you anticipate that you will have time to take a look at this review? Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

lheagy commented 5 years ago

:wave: @nhejazi, would you be willing to join this review?

nhejazi commented 5 years ago

Sure, would be happy to provide a review!

lheagy commented 5 years ago

@whedon add @nhejazi as reviewer

whedon commented 5 years ago

OK, @nhejazi is now a reviewer

lheagy commented 5 years ago

Many thanks @nhejazi! I have added a checklist to the main issue thread above to help guide your review. Please let me know if you have any questions!

nhejazi commented 4 years ago

@elyiorgos, the sleeppy package looks interesting, thanks for submitting the paper to JOSS! After going through the reviewer checklist, there are a few outstanding items that I'd like to bring your attention:

elyiorgos commented 4 years ago

Hello Nima!

Thank you for your comments!

In the current sleeppy github repo there should be an existing CONTRIBUTING.md file that outlines the ways in which any interested party can contribute if they would like to. If that file needs more fleshing out I'm happy to update it. I'm also happy to add a link to the README.md file to make it easier to find if that is a suitable and helpful addition.

With regards to a testing suite, there is a demo module with a demo.py file that runs the full suite of analyses on demo data, generates reports to be compared to the expected output(from a visual point of view), and also performs assertion tests on the expected clinical endpoints calculated by the package. My understanding in talking with the editor pre-submission was that those would suffice, however, I fully understand the importance of more robust testing, and am more than happy to create a suite of more conventional unit tests testing the many specific pieces of functionality leveraged by the package.

When you get a chance let me know your thoughts on both accounts, and I'll get right on it! Thank you again!

Wishing you all the best, Yiorgos Christakis

On Sun, Sep 29, 2019 at 12:20 AM Nima Hejazi notifications@github.com wrote:

@elyiorgos https://github.com/elyiorgos, the sleeppy package looks interesting, thanks for submitting the paper to JOSS! After going through the reviewer checklist, there are a few outstanding items that I'd like to bring your attention:

  • The package requires a statement about contribution guidelines. A simple approach for doing this includes adding a section "Contributing" to the README that links to a more complete document (e.g., CONTRIBUTING.md), giving those interested in adding to the package information on how best to do so.
  • From an examination of the package source, it appears that there are no unit tests included in the package. There are several frameworks for writing such tests, including unittest https://docs.python.org/2/library/unittest.html, nose2 https://github.com/nose-devs/nose2, and pytest https://pytest.readthedocs.io/en/latest/. Such tests are critical for ensuring software integrity as development continues. If I missed a note about tests or they are located elsewhere, feel free to point this out also.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/1663?email_source=notifications&email_token=AECHRMD7FHO5DF6FYBXOH5TQMAUHLA5CNFSM4INF6THKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD73H2LQ#issuecomment-536247598, or mute the thread https://github.com/notifications/unsubscribe-auth/AECHRMFVNPYDW6WFDRIBYTLQMAUHLANCNFSM4INF6THA .

nhejazi commented 4 years ago

Hi Yiorgos ---

Thanks for your quick reply and for pointing out the items that I missed. The included CONTRIBUTING.md file looks good to me --- please do link to it from the README.md though as this will make it much easier for interested users to find the document. Also, you're right that formal unit tests are technically not necessary to pass the JOSS review, that is, the checks currently included in the demo.py file appear sufficient. I will check off the testing item as it appears that the included checks do satisfy the requirement; however, I'd still strongly encourage adding unit tests in the style of whichever framework you prefer in the future as this will go a long way towards ensuring reproducibility as changes to the core software are made (possibly by contributors). Once such tests are set up, you may also consider looking into leveraging existing continuous integration services to ensure that your tests are run upon each commit / PR (you could also set this up with the existing demo.py and perhaps check that key numerical results are consistent between commits / PRs). Happy to check off the remaining item about contributions as well after the README.md is updated. Almost there!

Cheers, Nima

elyiorgos commented 4 years ago

Hi Nima,

Thanks for taking a look at what I mentioned. I went ahead and updated the the README to include a link to the contributions document. I completely agree with your testing point, robust unit testing and integration tests for the project are at the top of our to do list in terms of development. Thanks again for your help and responsiveness! We're very excited!

Best, Yiorgos

On Mon, Sep 30, 2019 at 10:24 PM Nima Hejazi notifications@github.com wrote:

Hi Yiorgos ---

Thanks for your quick reply and for pointing out the items that I missed. The included CONTRIBUTING.md file looks good to me --- please do link to it from the README.md though as this will make it much easier for interested users to find the document. Also, you're right that formal unit tests are technically not necessary to pass the JOSS review, that is, the checks currently included in the demo.py file appear sufficient. I will check off the testing item as it appears that the included checks do satisfy the requirement; however, I'd still strongly encourage adding unit tests in the style of whichever framework you prefer in the future as this will go a long way towards ensuring reproducibility as changes to the core software are made (possibly by contributors). Once such tests are set up, you may also consider looking into leveraging existing continuous integration services to ensure that your tests are run upon each commit / PR (you could also set this up with the existing demo.py and perhaps check that key numerical results are consistent between commits / PRs). Happy to check off the remaining item about contributions as well after the README.md is updated. Almost there!

Cheers, Nima

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/1663?email_source=notifications&email_token=AECHRMD4PVBDNATJI5ZXUWTQMKYFHA5CNFSM4INF6THKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD77WKLI#issuecomment-536831277, or mute the thread https://github.com/notifications/unsubscribe-auth/AECHRMD6VVHIQKYOR6KHXIDQMKYFHANCNFSM4INF6THA .

lheagy commented 4 years ago

Many thanks @nhejazi for your speedy review!

@elyiorgos: if you are able to include a test suite, that would be preferable for publication. Although having examples run does act as a test, that really is the bare minimum.

lheagy commented 4 years ago

:wave: Just checking in @eskofier. How are things going? In order to check-off the boxes for the review, you will need to have accepted the JOSS invitation: https://github.com/openjournals/joss-reviews/invitations. Please let me know if you have any questions. Thanks for your work on this!

ikarosilva commented 4 years ago

Hi Guys, sorry for the delay. Update my review list. My feedback:

1) I did not see any unit tests (am I missing something)? Would be nice to have a standard unit test folder that uses python unit test frameworks (nose etc).

2) Please avoid the use of wild imports such as:

from sleeppy.sleep import * Instead, limit the scope of your import to only those functions/classes necessary.

eskofier commented 4 years ago

Dear all,

I took a look at this with my senior PhD student Stefan Gradl (github: gradlman, also CCed). Feedback:

-> the code looks well documented where needed -> we could not really test the code because of its programming in python 2.7. I don’t have a 2.7 installation anymore, neither does Stefan or most people in my lab (except 1). Stefan tried to get the code running for half a day by downloading old packages or searching for compatible ones, but without success. As Python 2.7 will not be supported anymore after the end of this year [1, 2], we decided to not try further.

[1] https://pythonclock.org [2] https://www.anaconda.com/end-of-life-eol-for-python-2-7-is-coming-are-you-ready/

Please let us know how to proceed.

Best regards, Bjoern Eskofier -- Prof. Bjoern Eskofier, Ph.D.

FAU Sandbox: find out more about our newest FAU student startup & innovation program at www.sandbox.fau.de

Heisenberg professorship of the DFG Digital Sports and Health Group Machine Learning and Data Analytics Lab Department of Computer Science Faculty of Engineering Friedrich-Alexander University Erlangen-Nuernberg (FAU) Carl-Thiersch-Strasse 2b, 91052 Erlangen, Germany

Adjunct Member, Faculty of Medicine Speaker, Central Institute of Healthcare Engineering Friedrich-Alexander University Erlangen-Nuernberg (FAU) Henkestraße 91, 91052 Erlangen

I-net: http://www.mad.cs.fau.de Phone: +49 9131 85-27297 Email: bjoern.eskofier@fau.de

Am 07.10.2019 um 00:34 schrieb Ikaro Silva notifications@github.com:

Hi Guys, sorry for the delay. Update my review list. My feedback:

• I did not see any unit tests (am I missing something)? Would be nice to have a standard unit test folder that uses python unit test frameworks (nose etc).

• Please avoid the use of wild imports such as:

from sleeppy.sleep import * Instead, limit the scope of your import to only those functions/classes necessary.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

elyiorgos commented 4 years ago

Hello all,

I'll try to address things in order, though forgive me if I miss something.

@nhejazi Thank you for your quick feedback, and quick responses!

@lheagy With regards to a testing suite, without getting into any unnecessary details, the research project that helped spawn the package is currently being overhauled from a testing point of view, which will eventually require the addition of a full testing suite to the package, though this may take some time as other pipelines take priority.

@ikarosilva Thank you for your comments! I'll be sure to fix the wild import you showed, as well as any other similar instance across the package. As far as testing goes, the demo folder provides dummy data that can be automatically run through the package, producing reports and clinical endpoints for one night's sleep. When the demo is run, there are assertion tests that run through the produced endpoints and confirm that they match the expected output. I realize this is more of a full package integration test, and a full suite of unit tests in addition to that is desirable to verify that the package functions perform as expected, though as I mentioned above that may take some time. As written, the package does meet the minimum testing requirements for publication, though I understand if that causes you(or others) to have reservations in your review.

@eskofier I'm sorry to hear you had difficulty getting the package to install and run. Assuming you have access to an apple computer, python 2.7 should be built-in and the sleeppy package can simply be pip-installed. The pip installation is written in such a way that it installs all required dependencies with the correct versions, which should be all you need to be able to run the demo/test from inside the python interpreter (more detailed run directions are available in the readme). I'm happy to provide any assistance beyond that if necessary, please let me know.

A conversion of the package to python 3 will soon be in the works (for exactly the reasons you mentioned), but will take a back seat to fleshing out testing, so the timeline for that will be somewhat more extended. Our choice of python 2.7 was related to the fact that the package wasn't developed in a vacuum, but had to be integrated with a large existing project code base that had python 2.7 dependencies.

Please let me know if there are any more questions or comments, and thank you all for your help evaluating the package.

Best, Yiorgos Christakis

eskofier commented 4 years ago

Hi,

pip install did not work on an apple computer

Bjoern$ pip install sleeppy Collecting sleeppy ERROR: Could not find a version that satisfies the requirement sleeppy (from versions: none) ERROR: No matching distribution found for sleeppy

Manual install did not work

error: Setup script exited with error: library dfftpack has Fortran sources but no Fortran compiler found (as well as several other errors)

Sorry, but I do not have time to mess with technology. I have a couple of hours of work ahead of me and it’s 10p

Viele Grüße Bjoern

Am 08.10.2019 um 21:36 schrieb Yiorgos Christakis notifications@github.com:

Hello all,

I'll try to address things in order, though forgive me if I miss something.

@nhejazi Thank you for your quick feedback, and quick responses!

@lheagy With regards to a testing suite, without getting into any unnecessary details, the research project that helped spawn the package is currently being overhauled from a testing point of view, which will eventually require the addition of a full testing suite to the package, though this may take some time as other pipelines take priority.

@ikarosilva Thank you for your comments! I'll be sure to fix the wild import you showed, as well as any other similar instance across the package. As far as testing goes, the demo folder provides dummy data that can be automatically run through the package, producing reports and clinical endpoints for one night's sleep. When the demo is run, there are assertion tests that run through the produced endpoints and confirm that they match the expected output. I realize this is more of a full package integration test, and a full suite of unit tests in addition to that is desirable to verify that the package functions perform as expected, though as I mentioned above that may take some time. As written, the package does meet the minimum testing requirements for publication, though I understand if that causes you(or others) to have reservations in your review.

@eskofier I'm sorry to hear you had difficulty getting the package to install and run. Assuming you have access to an apple computer, python 2.7 should be built-in and the sleeppy package can simply be pip-installed. The pip installation is written in such a way that it installs all required dependencies with the correct versions, which should be all you need to be able to run the demo/test from inside the python interpreter (more detailed run directions are available in the readme). I'm happy to provide any assistance beyond that if necessary, please let me know.

A conversion of the package to python 3 will soon be in the works (for exactly the reasons you mentioned), but will take a back seat to fleshing out testing, so the timeline for that will be somewhat more extended. Our choice of python 2.7 was related to the fact that the package wasn't developed in a vacuum, but had to be integrated with a large existing project code base that had python 2.7 dependencies.

Please let me know if there are any more questions or comments, and thank you all for your help evaluating the package.

Best, Yiorgos Christakis

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

yiorg commented 4 years ago

Hi Bjoern,

I understand your frustration, and I'm sorry this is adding to an already demanding schedule. My guess is that since your laptop is probably rather new and you yourself use python 3, it's likely that the default python environment is python 3, so when you run a normal pip command like you showed above, it tries to pull a distribution for python 3 and can't find one. I actually ran into the same issue on my personal laptop, but if you run pip2 install sleeppy it should download and set up without a hitch. On my personal I had to run sudo pip2 install sleeppy due to a permissions error. After that, if you choose to run it from the python interpreter, just make sure that when starting python in the command line you run python2 as opposed to just python. Again that should work, but if not please let me know. If necessary I'm more than happy to set up a call/chat with either you or your colleague to sort things out. Thank you again for agreeing to review the package, and for your feedback.

Best, Yiorgos

eskofier commented 4 years ago

Hi Yiorgos,

Did not work :/ When I run:

$ pip2 install sleeppy -bash: pip2: command not found

Also:

$ pip3 install sleeppy Collecting sleeppy ERROR: Could not find a version that satisfies the requirement sleeppy (from versions: none) ERROR: No matching distribution found for sleeppy

Like before

$ pip install sleeppy Collecting sleeppy ERROR: Could not find a version that satisfies the requirement sleeppy (from versions: none) ERROR: No matching distribution found for sleeppy

What should we try next?

Viele Grüße Bjoern

Am 11.10.2019 um 17:54 schrieb Yiorgos Christakis notifications@github.com:

Hi Bjoern,

I understand your frustration, and I'm sorry this is adding to an already demanding schedule. My guess is that since your laptop is probably rather new and you yourself use python 3, it's likely that the default python environment is python 3, so when you run a normal pip command like you showed above, it tries to pull a distribution for python 3 and can't find one. I actually ran into the same issue on my personal laptop, but if you run pip2 install sleeppy it should download and set up without a hitch. On my personal I had to run sudo pip2 install sleeppy due to a permissions error. After that, if you choose to run it from the python interpreter, just make sure that when starting python in the command line you run python2 as opposed to just python. Again that should work, but if not please let me know. If necessary I'm more than happy to set up a call/chat with either you or your colleague to sort things out. Thank you again for agreeing to review the package, and for your feedback.

Best, Yiorgos

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

elyiorgos commented 4 years ago

Hi Bjoern,

Are you able to run python 2 from the command line with the command python2? If so you can install pip2 by running the following:

This will get you what you need to download pip wget https://bootstrap.pypa.io/get-pip.py

This will install pip for python 2 sudo python2 get-pip.py

This will install pip for python 3 sudo python3 get-pip.py

Now if you call pip, or pip3, you will get pip for python 3, and if you call pip2, you'll get pip for python 2. From there running pip2 install sleeppy should work.

If running python2 doesn't work for you for whatever reason, let me know and we can continue from there.

Best, Yiorgos

lheagy commented 4 years ago

@elyiorgos and @eskofier: If getting up and running locally is proving to be challenging, then I would recommend that @elyiorgos set up an example that runs on binder. You will need to specify an environment.yml file that contains the dependencies, and binder will install those for you and give you a sandbox computational environment to run the code. See for example this demo repository: https://github.com/binder-examples/python-conda_pip.

Creating a binder demonstrates that the software can be installed, and doesn't mess with @elyiorgos's software environment.

Please let me know if you run into any challenges getting up and running with binder.

eskofier commented 4 years ago

Are you able to run python 2 Yes, that works.

wget https://bootstrap.pypa.io/get-pip.py Doesn’t work: -bash: wget: command not found

Viele Grüße Bjoern

Social Media: find out more about

FAU Sandbox www.sandbox.fau.de https://twitter.com/FauSandbox https://www.instagram.com/fausandbox https://www.linkedin.com/company/fau-sandbox https://www.facebook.com/FAUSandbox

MaD Lab www.mad.tf.fau.de https://twitter.com/fau_mad_lab https://twitter.com/bjoerneskofier www.linkedin.com/in/bjoerneskofier https://de-de.facebook.com/FAU.MaD.Lab

Am 14.10.2019 um 17:29 schrieb Yiorgos Christakis notifications@github.com:

Hi Bjoern,

Are you able to run python 2 from the command line with the command python2? If so you can install pip2 by running the following:

This will get you what you need to download pip wget https://bootstrap.pypa.io/get-pip.py

This will install pip for python 2 sudo python2 get-pip.py

This will install pip for python 3 sudo python3 get-pip.py

Now if you call pip, or pip3, you will get pip for python 3, and if you call pip2, you'll get pip for python 2. From there running pip2 install sleeppy should work.

If running python2 doesn't work for you for whatever reason, let me know and we can continue from there.

Best, Yiorgos

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

yiorg commented 4 years ago

@lheagy Thank you for the suggestion, that seems like the best way forward. I'll get working on it as soon as possible and share it here once I have it up and running!

Best, Yiorg

elyiorgos commented 4 years ago

@lheagy @eskofier Thank you for your patience guys, the binder example is up and running! It's accessible from the readme in the demo section, but the following link should work as well:

https://mybinder.org/v2/gh/elyiorgos/sleeppy/master?filepath=example_notebook%2Fexample.ipynb

The environment may take a little while to set up (5-10 minutes maybe), but the version of the demo I have posted has all the data preloaded, so that portion should run quickly. In the results folders the pdfs that are generated don't open in browser for me (no idea why), but if you download them to local you can see them just fine.

Let me know if you have any issues, thanks again! Yiorg

lheagy commented 4 years ago

:wave: Hi @eskofier, just checking in. Have you had a chance to check out the binder link posted above? This should give you an environment where you can run the examples and play with the code-base.

eskofier commented 4 years ago

Hi, I will try to have a look the next few days.

Viele Grüße Bjoern

Am 05.11.2019 um 18:03 schrieb Lindsey Heagy notifications@github.com:

👋 Hi @eskofier, just checking in. Have you had a chance to check out the binder link posted above? This should give you an environment where you can run the examples and play with the code-base.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

eskofier commented 4 years ago

Dear all,

I tried the demo, but got stuck - I am copying in the output of the Notebook below, and also provide a screenshot what happens when I "Simply copy the url above, and replace /example.ipynb with /demo to load the demo folder.“. How should we proceed?

—————> Notebook output:

Example sleeppy usage: running the demo This short tutorial serves to confirm that sleeppy installs and runs correctly. In [1]:

We first import the demo from sleeppy.

import sleeppy.tests as demo In [2]:

next we get the current working directory

import os print(os.getcwd()) /home/jovyan/example_notebook

In [*]:

The next step is to run the demo and provide the current working directory as the results directory

demo.run_demo(binder_demo=True) Please provide a path to a results directory: /Users/Bjoern/Desktop/Joss

Your previous entry was not appropriate. It should follow a format similar to /home/jovyan/example_notebook Please provide a path to a results directory:

Now you can go to the generated results and explore sleeppy's outputs Simply copy the url above, and replace /example.ipynb with /demo to load the demo folder.

—————> End notebook output

Viele Grüße Bjoern

Am 24.10.2019 um 17:16 schrieb Yiorgos Christakis notifications@github.com:

@lheagy @eskofier Thank you for your patience guys, the binder example is up and running! It's accessible from the readme in the demo section, but the following link should work as well:

https://mybinder.org/v2/gh/elyiorgos/sleeppy/master?filepath=example_notebook%2Fexample.ipynb

The environment may take a little while to set up (5-10 minutes maybe), but the version of the demo I have posted has all the data preloaded, so that portion should run quickly. In the results folders the pdfs that are generated don't open in browser for me (no idea why), but if you download them to local you can see them just fine.

Let me know if you have any issues, thanks again! Yiorg

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

elyiorgos commented 4 years ago

Hi Bjoern,

Thanks for giving it a shot! The demo failed because the directory you provided wasn't the current working directory. Since the binder environment is one hosted online, if you provide a local directory it fails and prompts you to use a directory with the proper format and location (usually /home/jovyan/example_notebook). That's the reason the current working directory is printed in the previous statement, so it can be copy pasted easily as input.

So just to be 100% clear, when you run the demo and it prompts you with:

Please provide a path to a results directory:

You should just copy the current working directory printed above, (likely: /home/jovyan/example_notebook) and continue with that.

Let me know if you have any more issues, thank you!

Best, Yiorgos

eskofier commented 4 years ago

Dear All,

This worked, the results are partially unclear though - the pdf don’t work? Could you provide an a bit more in-depth insight into what happens during the demo, what the input data is and what is outputtet. Also, the python version issue remains. What is the opinion of the editor and the other reviewer on this?

Viele Grüße Bjoern

Am 13.11.2019 um 16:00 schrieb Yiorgos Christakis notifications@github.com:

Hi Bjoern,

Thanks for giving it a shot! The demo failed because the directory you provided wasn't the current working directory. Since the binder environment is one hosted online, if you provide a local directory it fails and prompts you to use a directory with the proper format and location (usually /home/jovyan/example_notebook). That's the reason the current working directory is printed in the previous statement, so it can be copy pasted easily as input.

So just to be 100% clear, when you run the demo and it prompts you with:

Please provide a path to a results directory:

You should just copy the current working directory printed above, (likely: /home/jovyan/example_notebook) and continue with that.

Let me know if you have any more issues, thank you!

Best, Yiorgos

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

elyiorgos commented 4 years ago

Hi Bjoern,

Happy to hear it worked!

With respect to the pdf's I think I had mentioned somewhere in an earlier message in the thread that for some reason the pdf's aren't viewable in browser, to see them you have to download them locally unfortunately.

In terms of what happens during the demo/during any given run of the package I would refer you to the readme (or the paper itself) for a more detailed description of what's going on in general, but in short:

With respect to versioning I will chime in and say that the python 3 implementation is essentially finished (barring some validation and documentation of some of the implemented updates), the python 2 implementation will be retired, but frozen, and a more comprehensive and thorough testing suite will accompany the python 3 version (using pytest as a framework).

I hope that helped clear things up, please let me know if you have any more questions. Thank you!

Best, Yiorgos

elyiorgos commented 4 years ago

Hi all, just checking in to see if there's anything missing or anything I can clear up. Thanks again for taking the time. For those of you celebrating Thanksgiving soon I hope you enjoy the break!

Best, Yiorgos

eskofier commented 4 years ago

Dear other reviewers and editor,

I did ask about the version issue - if the others don‘t see this as problematic, Ineould be OK with accepting overall.

Best regards, Bjoern

-sent from mobile office-

Am 26.11.2019 um 22:04 schrieb Yiorgos Christakis notifications@github.com:

 Hi all, just checking in to see if there's anything missing or anything I can clear up. Thanks again for taking the time. For those of you celebrating Thanksgiving soon I hope you enjoy the break!

Best, Yiorgos

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

lheagy commented 4 years ago

Thanks @eskofier! Have you had a chance to take a look at the check-boxes in the main issue description? If you are satisfied with their current state, could you please tick off the boxes under your name? If you are unable to check the boxes, you might need to accept the invitation at: https://github.com/openjournals/joss-reviews/invitations

eskofier commented 4 years ago

Hi,

I just read through all the boxes and I would tick them all off if I could - how do I do that (on an iPhone)? I just spent 15 minutes searching and trying.

Best regards, Bjoern

-sent from mobile office-

Am 04.12.2019 um 00:56 schrieb Lindsey Heagy notifications@github.com:

 Thanks @eskofier! Have you had a chance to take a look at the check-boxes in the main issue description? If you are satisfied with their current state, could you please tick off the boxes under your name? If you are unable to check the boxes, you might need to accept the invitation at: https://github.com/openjournals/joss-reviews/invitations

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

lheagy commented 4 years ago

Thanks @eskofier, with your written confirmation, I can tick them off for you.

lheagy commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago 
Attempting PDF compilation. Reticulating splines etc...
whedon commented 4 years ago

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

yiorg commented 4 years ago

Thank you everyone!

From what I can tell the article proof looks good. Please let me know about any next steps on our end.

Best, Yiorgos

lheagy commented 4 years ago

@whedon check references