[REVIEW]: The MTPy software package for magnetotelluric data analysis and visualisation

whedon commented 5 years ago

Submitting author: @alkirkby (Alison Kirkby) Repository: https://github.com/MTgeophysics/mtpy Version: v1.0 Editor: @lheagy Reviewer: @JKutt Archive: 10.5281/zenodo.2698634

Status

Status badge code:

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

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

@JKutt, 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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @lheagy know.

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

Review checklist for @JKutt

Conflict of interest

[x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

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] Version: v1.0
[x] Authorship: Has the submitting author (@alkirkby) 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)?

whedon commented 5 years ago

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

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

whedon commented 5 years ago

Attempting PDF compilation. Reticulating splines etc...

whedon commented 5 years ago

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

/app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/lib/whedon.rb:83:in check_fields': Paper YAML header is missing expected fields: date (RuntimeError) from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/lib/whedon.rb:69:ininitialize' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/lib/whedon/processor.rb:32:in new' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/lib/whedon/processor.rb:32:inset_paper' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/bin/whedon:55:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:inrun' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:indispatch' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/bin/whedon:116:in<top (required)>' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

'

lheagy commented 5 years ago

:wave: Many thanks @JKutt 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#1358 in the issue text.

The author @alkirkby mentioned that she will fix the images in the paper on monday.

Please let me know if you have any questions!

alkirkby commented 5 years ago

@whedon generate pdf

whedon commented 5 years ago

Attempting PDF compilation. Reticulating splines etc...

whedon commented 5 years ago

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

/app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/lib/whedon.rb:83:in check_fields': Paper YAML header is missing expected fields: date (RuntimeError) from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/lib/whedon.rb:69:ininitialize' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/lib/whedon/processor.rb:32:in new' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/lib/whedon/processor.rb:32:inset_paper' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/bin/whedon:55:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:inrun' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:indispatch' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-96d847bd9060/bin/whedon:116:in<top (required)>' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

'

alkirkby commented 5 years ago

@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:

alkirkby commented 5 years ago

@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:

JKutt commented 5 years ago

Hi @lheagy, I managed to complete my review! I had a bit of a stumble at first getting things running but after following the Linux install more thoroughly I managed to get it going. Once up and running everything went quite smooth! A lot of good stuff in here. I found a few things and created an issue for one but the others already exist (e.g. error with print statements due to python 2 vs. 3).

alkirkby commented 5 years ago

Hey @JKutt, thanks for the review! We're glad you managed to install MTPy in the end, but we are tidying up some of the issues at the moment to make it easier for users. I was wondering if you could let me know the issue numbers for the issues you mentioned above? I can't find anything in the issues page about print statements with python 2 vs 3, for example. I need to go through the issues page as some of the issues were resolved but never closed.

lheagy commented 5 years ago

Many thanks for your speedy review @JKutt! @alkirkby: it looks they are issues

MTgeophysics/mtpy#77
MTgeophysics/mtpy#78

alkirkby commented 5 years ago

Great thanks @lheagy. I've checked both those issues - #78 has been addressed and I have now addressed #77.

JKutt commented 5 years ago

Hi @alkirkby the "print" statements issue was brought up in MTgeophysics/mtpy#63

alkirkby commented 5 years ago

Hi @JKutt, Fei tells me that the print statements issue was resolved, are you able to specify where in MTPy you're encountering these issues?

JKutt commented 5 years ago

Hi @JKutt, Fei tells me that the print statements issue was resolved, are you able to specify where in MTPy you're encountering these issues?

@alkirkby sorry for the late reply, missed this one. I can't quite remember off the top of my head but it was in places where "print" is used instead of "print()"

zhang01GA commented 5 years ago

Hi @JKutt and @alkirkby: Two weeks ago, I did fix one new print issue in mtpy/imaging/plotstrike.py https://github.com/MTgeophysics/mtpy/commit/c4b6f3d8ffac3636a9bb60e7fc072bffcf312f73

Today, I applied 2to3 to every *.py script in mtpy. But had not found any print statement issue. All active print statement has been changed to print() in the refactoring long time ago. (some py2 prints may exist in commented out # lines. But they are not active).

Please let me know. Thanks

lheagy commented 5 years ago

Thanks @zhang01GA! @JKutt, would you mind taking another look when you have a chance?

JKutt commented 5 years ago

Thanks @zhang01GA! @JKutt, would you mind taking another look when you have a chance?

All seems good now! No issues second time around.

lheagy commented 5 years ago

👋 @JKutt: it looks like the box for automated-tests is still un-checked. Do you have any other comments wrt testing? or can this be ticked off? Thanks!

JKutt commented 5 years ago

👋 @JKutt: it looks like the box for automated-tests is still un-checked. Do you have any other comments wrt testing? or can this be ticked off? Thanks!

@lheagy darn, I missed that one! Apologies. No concern on the testing, simply missed ticking it off. Should be good now!

lheagy commented 5 years ago

@whedon check references

whedon commented 5 years ago

Attempting to check references...

whedon commented 5 years ago


OK DOIs

- 10.1111/j.1365-246X.2004.02281.x is OK
- 10.1111/j.1365-246X.2004.02203.x is OK
- 10.1029/JB092iB01p00633 is OK
- 10.1190/1.1442813 is OK
- 10.1111/j.1365-246X.2011.05347.x is OK
- 10.1016/j.cageo.2014.01.010 is OK
- 10.1016/J.CAGEO.2014.07.013 is OK
- 10.1190/1.1438799 is OK
- 10.1111/j.1365-246X.1962.tb02992.x is OK

MISSING DOIs

- https://doi.org/10.1111/j.1365-246x.2005.02779.x may be missing for title: Determinable and non-determinable parameters of galvanic distortion in magnetotellurics

INVALID DOIs

- None

lheagy commented 5 years ago

Many thanks @JKutt for your review!!

@alkirkby, just a couple minor things to address before we proceed with publication.

It looks like there is one missing DOI in the paper:

https://doi.org/10.1111/j.1365-246x.2005.02779.x may be missing for title: Determinable and non-determinable parameters of galvanic distortion in magnetotellurics

Also, I posted an issue about the documentation website - currently, it looks like the auto-documentation isn't functioning as expected (MTgeophysics/mtpy#80). Please take a look when you have a chance.

alkirkby commented 5 years ago

@whedon check references

whedon commented 5 years ago

Attempting to check references...

whedon commented 5 years ago


OK DOIs

- 10.1111/j.1365-246x.2005.02779.x is OK
- 10.1111/j.1365-246X.2004.02281.x is OK
- 10.1111/j.1365-246X.2004.02203.x is OK
- 10.1029/JB092iB01p00633 is OK
- 10.1190/1.1442813 is OK
- 10.1111/j.1365-246X.2011.05347.x is OK
- 10.1016/j.cageo.2014.01.010 is OK
- 10.1016/J.CAGEO.2014.07.013 is OK
- 10.1190/1.1438799 is OK
- 10.1111/j.1365-246X.1962.tb02992.x is OK

MISSING DOIs

- None

INVALID DOIs

- None

alkirkby commented 5 years ago

Hi @lheagy, okay great!

@zhang01GA is looking into the documentation issue and will link the user guide from the main page. I've just fixed the doi issue. Nearly there!

alkirkby commented 5 years ago

Hi @lheagy, @zhang01GA has now fixed the documentation issue and has added a link to the user guide to the front page.

lheagy commented 5 years ago

Excellent, thanks @alkirkby!

Please take a look at the pdf proof below. If everything looks good, then please make a new release and archive the software on zenodo or similar and ensure that the title and author list match those in the paper. Then, please post the new version number and doi here.

lheagy commented 5 years ago

@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:

alkirkby commented 5 years ago

Great thanks @lheagy, I've just created a release and archived the software: DOI: 10.5281/zenodo.2698634 We are now just awaiting final sign-off of the proof. We'll let you know when it's ready to go!

alkirkby commented 5 years ago

Hi @lheagy, We have just obtained final approval of the proof so are ready to go ahead with the next stage of the process!

alkirkby commented 5 years ago

version number is 1.0

lheagy commented 5 years ago

@whedon set 10.5281/zenodo.2698634 as archive

whedon commented 5 years ago

OK. 10.5281/zenodo.2698634 is the archive.

lheagy commented 5 years ago

@whedon set 1.0 as version

whedon commented 5 years ago

OK. 1.0 is the version.

lheagy commented 5 years ago

@whedon check references

whedon commented 5 years ago

Attempting to check references...

whedon commented 5 years ago


OK DOIs

- 10.1111/j.1365-246x.2005.02779.x is OK
- 10.1111/j.1365-246X.2004.02281.x is OK
- 10.1111/j.1365-246X.2004.02203.x is OK
- 10.1029/JB092iB01p00633 is OK
- 10.1190/1.1442813 is OK
- 10.1111/j.1365-246X.2011.05347.x is OK
- 10.1016/j.cageo.2014.01.010 is OK
- 10.1016/J.CAGEO.2014.07.013 is OK
- 10.1190/1.1438799 is OK
- 10.1111/j.1365-246X.1962.tb02992.x is OK

MISSING DOIs

- None

INVALID DOIs

- None

lheagy commented 5 years ago

@whedon set v1.0 as version

whedon commented 5 years ago

OK. v1.0 is the version.

openjournals / joss-reviews