[REVIEW]: rsudp: A Python package for real-time seismic monitoring with Raspberry Shake instruments

whedon commented 4 years ago

Submitting author: @iannesbitt (Ian Nesbitt) Repository: https://github.com/raspishake/rsudp Version: 1.1.0 Editor: @jedbrown Reviewers: @fwalter, @calum-chamberlain Archive: 10.5281/zenodo.5771026

: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 badge code:

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

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

@calum-chamberlain and @fwalter, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

Make sure you're logged in to your GitHub account
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 @jedbrown know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Review checklist for @calum-chamberlain

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS 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] Contribution and authorship: Has the submitting author (@iannesbitt) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[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] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] 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?

Review checklist for @fwalter

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS 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] Contribution and authorship: Has the submitting author (@iannesbitt) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[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] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] 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?

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @eileenrmartin 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:

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

watching

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.1785/0220180251 is OK
- 10.1785/0220190211 is OK
- 10.3389/feart.2020.00009 is OK
- 10.3389/feart.2020.00073 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.1785/gssrl.81.3.530 is OK
- 10.4401/ag-4838 is OK
- 10.1088/1749-4699/8/1/014003 is OK

MISSING DOIs

- None

INVALID DOIs

- None

whedon commented 4 years ago

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

jedbrown commented 4 years ago

@eileenrmartin :wave: Welcome to JOSS and thanks for agreeing to review!

The comments from @whedon above outline the review process, which takes place in this thread (possibly with issues filed in the rsudp repository). I'll be watching this thread if you have any questions.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/2565 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within a month or so. Please let me know if you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@jedbrown) if you have any questions/concerns.

labarba commented 4 years ago

hi 👋 @eileenrmartin – I'm the associate EiC on rotation this week. I notice this review has no movement in several weeks. Could we have an update from you about your availability to complete the review? Thanks!

iannesbitt commented 3 years ago

note to reviewers, here is a potentially relevant recent citation that uses RS that might be good to include among the first few references cited:

https://science.sciencemag.org/content/369/6509/1338.abstract

jedbrown commented 3 years ago

@whedon add @fwalter as reviewer

whedon commented 3 years ago

OK, @fwalter is now a reviewer

jedbrown commented 3 years ago

@fwalter (Fabian Walter) :wave: Welcome to JOSS and thanks for agreeing to review!

The comments from @whedon above outline the review process, which takes place in this thread (possibly with issues filed in the rsudp repository). I'll be watching this thread if you have any questions.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/2565 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within a month or so. Please let me know if you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@jedbrown) if you have any questions/concerns.

fwalter commented 3 years ago

Dear Jed,

I have reviewed this software package and would now like to submit my report. Do I understand correctly that this should be done as an issue on the repository:

https://github.com/raspishake/rsudp/issues

Also, where can I fill out the checklist, which is mentioned here:

https://joss.readthedocs.io/en/latest/review_checklist.html

I tried to accept the invitation by whedon, but this seems to have expired. Is that a problem?

In general, this software package compiles and functions well, but it seems to be mostly (perhaps only) useful for educational purposes. For research or monitoring purposes one has to include custom modules, which I found tedious, especially when the user is not highly fluent in python.

Thanks for your help! Fabian.

Am Mi., 18. Nov. 2020 um 02:17 Uhr schrieb Jed Brown < notifications@github.com>:

@fwalter https://github.com/fwalter (Fabian Walter) 👋 Welcome to JOSS and thanks for agreeing to review!

The comments from @whedon https://github.com/whedon above outline the review process, which takes place in this thread (possibly with issues filed in the rsudp repository https://github.com/raspishake/rsudp/). I'll be watching this thread if you have any questions.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention #2565 https://github.com/openjournals/joss-reviews/issues/2565 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within a month or so. Please let me know if you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@jedbrown https://github.com/jedbrown) if you have any questions/concerns.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/2565#issuecomment-729313235, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADC2IN53Q3FSUPMDTIBKK3DSQMODFANCNFSM4P7RIADA .

--

Fabian Walter ETH Zurich Laboratory of Hydraulics, Hydrology and Glaciology V. Wasserbau, Hydrologie u. Glaz. HIA D 56.1 Hönggerbergring 26 8093 Zürich

Phone: +41 44 632 4162 http://n.ethz.ch/~fwalter/

jedbrown commented 3 years ago

@whedon re-invite @fwalter as reviewer

whedon commented 3 years ago

OK, the reviewer has been re-invited.

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

jedbrown commented 3 years ago

@fwalter Thanks! When you accept that fresh invitation, you should be able to click in the checkboxes.

Indeed, general comments can go directly in this thread, but specific "to do" items should be filed as issues in https://github.com/raspishake/rsudp/issues. They'll be automatically linked from here if you mention this issue (https://github.com/openjournals/joss-reviews/issues/2565).

jedbrown commented 3 years ago

Hi, @eileenrmartin :wave: Is there anything we can do to facilitate your review?

fwalter commented 3 years ago

The submitted python-based rsupd package is designed to handle data output of the Raspberry Shake and Raspberry Boom sensors. Although I have not used these sensors before I understand that they are user friendly and low cost seismic and infrasound sensors particularly suitable for educational purposes or research applications, where specialized custom-made instrumentation is necessary.

The code can be installed and compiled with no problems and functionalities and python classes are documented well. As I do not own a Raspberry Shake or Raspberry Boom myself, I had to rely on the testing and demoing mode for this review. I have no reason to doubt that the package works well with the actual hardware, but I would suggest including a step-by-step tutorial on how to set up, handle and configure the software together with the hardware. The documentation does include a section on how to run the software in batch processing mode.

Beyond data reading from a Raspberry instrument, the software package does not seem to offer new scientific functionality compared to existing software. Spectrograms, amplitude statistics and STA/LTA detectors are standard in high-level seismic analysis toolboxes (e.g. obspy, GISMO, SEIZMO) or can be easily programmed from scratch. The twitter and telegram functionality can be a useful add on for communicating realtime processing output.

As outlined in the “Developer’s Guide”, the program architecture seems straightforward and appropriately designed. The definition and purpose of queues and threads and the difference behind these concepts may not be clear to all users. I suggest elaborating on this to allow a non-software-expert grasp the operation of rsupd better.

From the paper it is not clear how this software compares with other packages (hence the unchecked box in the Review List). There is a paragraph on this in the documentation (“1.2. Why It’s Special”), however, the argumentation is very general. The code requirement that “calculations be made quickly” should be elaborated on in this regard (Which kind of calculations? How/to what extent can they already be done in other packages?).

From my point of view, this is a particularly helpful software package for teaching purposes. I can envision student exercises using this software package in combination with the Raspberry hardware. Beyond the classroom this could be useful for applications where Raspberry products run operationally and continuously. Personally, I would not switch to this approach, as for my research I stream and archive seismic data via professional national network agencies. For Raspberry users, the rsupd package may help data handling, but I found the described solution to include custom consumers somewhat difficult to use as several rsupd modules required modification (although the given instructions were clear and worked well). As described in issue #12 on the rsudp github page, it would be helpful to have a single function, which packages data into a python object, ideally an obspy stream object. Users should be able to hook into such an interface without detailed knowledge of the source code or expert programming knowledge. This would give access to the most advanced open source and tested code for analyzing seismic data and the obspy package is already used for making rsupd run, so this would be a natural choice.

In summary, this is a useful software package for educational purposes and customers of the Raspberry products. I expect Raspberry Shake/Boom users to employ and extend this package in their applications. If the data handling interface is simplified and improved (see issue #12 on the target repository), this software package will facilitate realtime data handling for Raspberry users.

Note: The references do not appear in the software paper.

fwalter commented 3 years ago

This concludes my review of this software submission.

jedbrown commented 3 years ago

Thanks, @fwalter. What do you mean by "The references do not appear in the software paper."? I see 8 references with DOI and nothing unresolved in the PDF.

I suppose I would mention that even if something can be easily coded from scratch, there's still value in having an authoritative implementation for provenance, accessibility, and sometimes performance. The ObsPy integration you suggest sounds like it would be useful.

@iannesbitt Please let us know if you have questions about revising to address the concerns above.

fwalter commented 3 years ago

Hi @jedbrown, I was referring to the paper.md file, which I view through my browser. Where can I see the pdf?

jedbrown commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

jedbrown commented 3 years ago

It was linked above in the thread, but here's a fresh version. ☝🏼

fwalter commented 3 years ago

Thanks!

iannesbitt commented 3 years ago

I have implemented the changes requested by @fwalter in raspishake/rsudp#13.

jedbrown commented 3 years ago

Great!

@eileenrmartin Could you let us know when you might be able to get to this? Is there anything other than time that we can assist you with?

iannesbitt commented 3 years ago

@jedbrown I wonder if it would make sense to email her, it seems like she may not be getting github notifications.

eileenrmartin commented 3 years ago

Review expected May 7. Sorry it seems I'm not getting any github notifications, so I didn't see any messages above.

osop-raspishake commented 3 years ago

Hi all! Just checking in from here in Panama to see how the review is coming along :) Thanks @eileenrmartin @iannesbitt

jedbrown commented 3 years ago

@whedon add @calum-chamberlain as reviewer

whedon commented 3 years ago

OK, @calum-chamberlain is now a reviewer

jedbrown commented 3 years ago

@whedon remove @eileenrmartin as reviewer

whedon commented 3 years ago

OK, @eileenrmartin is no longer a reviewer

jedbrown commented 3 years ago

@calum-chamberlain :wave: Welcome to JOSS and thanks for agreeing to review!

The comments from @whedon above outline the review process, which takes place in this thread (possibly with issues filed in the rsudp repository). I'll be watching this thread if you have any questions.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/2565 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within a month or so. Please let me know if you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@jedbrown) if you have any questions/concerns.

calum-chamberlain commented 3 years ago

This package provides a useful, simple to use and attractive real-time interface to RaspberryShake data, and I look forward to using it as an educational and outreach tool within my University and in broader communities. Firstly I would like to thank the developers for this tool. I do own a raspberry shake with a single-component sensor and have tested this with my own shake. Setup was simple and the instructions were easy to follow. I would have liked to see a link to setting up the UDP server on the shake from within the installation docs.

While the code seems to be of good quality, and has a nice simple test for functionality, it would be good to see unit-tests of some of the modular elements that could be run on CI systems to ensure that future changes to the code do not break any functionality. This may also make it easier for others to contribute extensions to this code, who may not be so familiar with all the elements of the package. On a similar note, I did not see any contributing guidelines on the github page, and would like to see a link to the Contributing to this project page from the github repository.

To me this code would be very helpful for education and outreach, and the statement of need in the online documentation is good, however I think the paper would benefit from taking some of the text from the Why it is special section of the docs to make the justification for the code more obvious. It would also be helpful to compare to other software for a similar purpose – I note that the “About” page of the documentation states that “Programs that do similar tasks are usually not as fully-featured, cost money, are unmaintained, or are complex to set up and run.” however it would be good to reference what those software are and have this short comparison statement in the paper.

As eluded to by @fwalter – being about to access data in an ObsPy Stream would be useful, although for most users I imagine that the standard configuration as offered now will be sufficient for their needs. However, for shakes connected to the internet data are available via FDSN, and this would be the natural route for collecting data for most RaspberryShake installs. It may however be useful to have access to an ObsPy Stream for storing data on a separate co-located machine in non-standard, not internet-enabled installs. I do not know how common such installs are though.

In summary, this is a nice visualisation tool for a specific set of seismic instruments that will likely be well-used by raspberry shake owners for outreach and education.

jedbrown commented 3 years ago

Thanks for your review, @calum-chamberlain. Could you clarify if/how you consider this useful for research, not strictly outreach and education? Are there parts of functionality or documentation that should be improved to better meet the needs of researchers?

calum-chamberlain commented 3 years ago

Hi @jedbrown - the research need of this software is not particularly clear to me given that data can be accessed from via seedlink and accessible via other tools, and hence through ObsPy as is commonly used by "power users". The ability to archive miniseed data locally is helpful, but as these continuous data are available to researchers via FDSN, alongside other data that would likely be useful for research, it isn't clear to me why I would want to archive data locally, unless (as suggested above) the raspberry shake installation was deployed without an internet connection and data were to be retrieved intermittently.

That this software provides a means to access the UDP Datacast is helpful, but it would be helpful to know if/why this is preferred to the seedlink stream.

A caveat to this is that my research needs, and therefore my view of this, might be narrow and missing something and I would be happy to be corrected!

iannesbitt commented 3 years ago

Hi @calum-chamberlain, thank you very much for the feedback. I have added nearly all of the suggestions you detail above, with the exception of ObsPy stream access which will have to wait for a future release. Hopefully the text I added to the Statement of Need is sufficient to detail rsudp's niche a little better. See raspishake/rsudp#22.

rsudp's research usefulness I believe is in the fact that it is lightweight and extensible and can be used to push notifications to various endpoints with very little data usage. Given limited internal storage and data restrictions, this could be useful for remote passive seismic projects that need to know when to retrieve data from their units but do not have the resources to pay for several GB/month of cellular data. The use of UDP rather than TCP is also purposeful to keep bandwidth usage down. The advantage over SeedLink is that there is no additional processing or storage read required on the part of the Shake OS, since UDP data packets are sent directly from a process in the Shake's software as data is being recorded whereas SeedLink data is provided by request. This keeps wear down slightly on the storage medium, which limits maintenance requirements for remote installations. The extensible nature of rsudp means that the STA/LTA trigger could also, for example, start a process to send event data to a server then shut it off after STA/LTA returns to normal.

If you think including this example would improve the paper, I can add it as well.

iannesbitt commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

iannesbitt commented 3 years ago

Fixed error with Winter (2021) citation.

iannesbitt commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

iannesbitt commented 3 years ago

Also @calum-chamberlain, I mentioned this in the linked issue but Scrutinizer CI runs the unit tests on each commit. You can see the results of this quickly by looking at the build status indicator on the main github page, or in detail by going to the build output page, clicking on "Node: tests" (on the right) then scrolling down to the output of rs-test

calum-chamberlain commented 3 years ago

Sorry I missed those tests @iannesbitt - can you point me towards where the unit tests themselves reside? I haven't found them yet. Thanks!

iannesbitt commented 3 years ago

Tests originate from the client and are defined in test.py.

calum-chamberlain commented 3 years ago

Thanks for that Ian - I had found that test file, but didn't think those to be unit tests - this may be a lack of understanding on my part, but the way I usually understand unit tests is a series of focused tests that test specific parts of the code base (similar to that outlined by the "agile alliance" here~searchTerm~'~sort~false~sortDirection~'asc~page~1)). Often this type of testing can be helpful to debug any issues, rather than having a fail caught somewhere in a systems test (as the main test seems to be) and needing to dig around to find the issue. Consider this to be a suggestion for future though, rather than a requirement.

The guidelines for review do state that it it desirable for the core functionality to be tested though, and it looks like some parts of the code-base do not have tests. For example, it doesn't look like the miniseed writing module is tested at the moment, and I don't think the data forwarding, twitter or telegram modules are tested either by the current tests? Again, happy to be corrected!

danielskatz commented 3 years ago

👋 @jedbrown - Can you provide an update on this submission and review?

jedbrown commented 3 years ago

Hi, @iannesbitt. Are you intending to leave the current state of testing or will you add tests. One important aspect is to ask what components other researchers might want to contribute to and make sure they are well tested. So if you have some component that does a well-defined task and will never change, then perhaps it doesn't need its own unit test. But a leaky abstraction or one with a researchy surface really needs tests to enable community contributions.

You're the expert on this particular code, but I would like to hear your response. Generating a coverage report is one of those "that which isn't measured never gets fixed" that's likely to be informative at very low effort (https://coverage.readthedocs.io/en/coverage-5.5/).

iannesbitt commented 3 years ago

Hi all, my apologies, I had intended to respond earlier but another paper submission got in the way. First of all yes @calum-chamberlain you are correct that those components don't get tested. Sorry for the misunderstanding. The reason I put off responding was that I wanted to take a look at what it would require to implement some simple functionality testing of the untested modules in the current routine. I think it would be possible without too much work, so I will have a go at it early next week.

@jedbrown I will look at implementing coverage tests as well.

jedbrown commented 3 years ago

Sounds great, thanks.

iannesbitt commented 3 years ago

Hi all, the requested changes have been added and merged to master. In particular:

all core modules are tested explicitly now, if applicable
code coverage is now implemented using python-cc (a badge has been added to represent this)

Apologies for the delay...I needed ample time to make sure this was done as thoroughly as possible. If I missed anything, let me know. Cheers and thank you again for your attention to detail.

openjournals / joss-reviews