Submit erddapy for review

[ocefpaf]

Submitting Author: Filipe Fernandes (@ocefpaf)
Repository Link: https://github.com/ioos/erddapy
Version submitted: v0.4.0
Editor: @lwasser
Reviewer 1: @jlpalomino @choldgraf
Reviewer 2: @npch
Archive: TBD
Version accepted: TBD

---

• Paste the full DESCRIPTION file inside a code block below:

erddapy is a python wrapper to ERDDAP's RESTful web services to create valid ERDDAP URLs for data requests, searches, metadata queries, etc.

Scope

• Please indicate which category or categories this package falls under:
  • Data retrieval
  • Data extraction
  • Data munging
  • Data deposition
  • Reproducibility
  • Geospatial
  • Education
  • Data visualization*

* Please fill out a pre-submission inquiry before submitting a data visualization package. For more info, see this section of our guidebook.

• Explain how the and why the package falls under these categories (briefly, 1-2 sentences):

erddapy is a tool to construct URLs for retrieving data from ERDDAP servers.

• Who is the target audience and what are scientific applications of this package?

The target audience is anyone who needs to access that in ERDDAP servers, they are usually Met/Ocean scientists who need near-real time access to data. erddapy does not have a direct scientific application b/c that varies based on the data available. It is essentially a retrieval tool. However, ERDDAP servers are essentially scientific data servers, erddapy's goal is to make access to that data easier.

• Are there other Python packages that accomplish the same thing? If so, how does yours differ?

Not to my knowledge.

• If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted:

Technical checks

For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:

• does not violate the Terms of Service of any service it interacts with.
• has an OSI approved license
• contains a README with instructions for installing the development version.
• includes documentation with examples for all functions.
• contains a vignette notebook with examples of its essential functions and uses.
• has a test suite.
• has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.

Publication options

• Do you wish to automatically submit to the Journal of Open Source Software? If so:

JOSS Options

• The package has an obvious research application according to JOSS's definition in their submission requirements. Be aware that completing the pyOpenSci review process does not guarantee acceptance to JOSS. Be sure to read their submission requirements (linked above) if you are interested in submitting to JOSS.
• The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
• The package is deposited in a long-term repository with the DOI:

Note: Do not submit your package separately to JOSS

Code of conduct

• I agree to abide by pyOpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

[lwasser]

@ocefpaf my apologies for missing this submission almost 2 weeks ago! in our meeting tomorrow, we will discuss who the reviewers will be for this submission. I will serve as the editor for it but also can review if need be!!

OPEN CALL: Does anyone want to review this submission?
Also just to confirm, Filipe, you are NOT interested in JOSS at this point in time?

[ocefpaf]

@ocefpaf my apologies for missing this submission almost 2 weeks ago! in our meeting tomorrow, we will discuss who the reviewers will be for this submission. I will serve as the editor for it but also can review if need be!!

No problem. This is a test submission after all 😄

BTW, one thing that comes to mind would be to set up a bot, similar to JOSS' bot, so the person submitting the package can get some communication as soon as s/he submits the PR. It can be something like: "Thanks for submitting, please read this and that while you wait for a review..."

OPEN CALL: Does anyone want to review this submission?

We could have a review team to ping here.
Something like @pyOpenSci/reviewers.

Also just to confirm, Filipe, you are NOT interested in JOSS at this point in time?

Not at this moment.

PS: I won't be able to make it to tomorrow's meeting.
I'm at AnacondaCon in case someone from the pyOpenSci team is here and wants to meet.

[lwasser]

@ocefpaf these are all great suggestions!! we will look into Joss' Bot. @kysolvik can you kindly look into what they have setup to provide an automated response??

I also really like the idea of having a set of reviewers that could be pinged. We might need to think that through given in the future we could have many submissions at a time. BUT let's keep talking as these are excellent suggestions.

enjoy the conference, Filipe!! We will catch you via zoom at the next meeting but i'm sure we will chat online before then.

[kysolvik]

JOSS has an excellent bot named Whedon: https://github.com/openjournals/whedon. It definitely seems to be pretty helpful. We could look into creating something similar!

In the mean time, it looks like rOpenSci just has the current editor in chief watch issues in their software-review repo and assign them as they come up. This could be an option until we're able to set up a bot.

[lwasser]

woo hoo! ok so i am going to serve as the editor for this submission. As a part of this process, i am reviewing the dev_guide closely and making edits / changes to it. pr soon to come. Below are the checks that i'm looking for

Editor checks:

• Fit: The package meets criteria for fit and overlap.
  DATA ACCESS (API WRAPPER - JOSS WOULD NOT REVIEW THIS i think??)
• Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
  TRAVIS is being used
• License: The package has an OSI accepted license
  BSD-3
• Repository: The repository link resolves correctly
• Archive (JOSS only, may be post-review): The repository DOI resolves correctly
• Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

---

Editor comments

This package is ready for review! @choldgraf @jlpalomino @npch are you still willing to serve as reviewers? Please note that a review could be technical in nature (looking at tests, code, etc etc) or could focus more on usability and documentation. i think ideally we'd hit both via all of our reviewers.

If you are still able to review, please let me know what time frame works for you? I was going to suggest 2-3 weeks. Does that sound reasonable? if so i'll select a date.

Update: following rOpenSci i am supposed to give you 3 weeks to review this package!! still going through the docs carefully. Does May 1 work for you guys as a deadline?

---

Reviewers: @choldgraf @jlpalomino @npch
Due date: 1 May 2019

[choldgraf]

Sounds reasonable to me - @jlpalomino how would you like to handle the joint review? Should we prepare it together then submit at once? Or we can just review independently and have a channel of communication somewhere? I'm happy w/ whatever

[jlpalomino]

@choldgraf I'd like to check out the review process docs on my own first, but then it would be great to prepare and submit one together. I'll send you an email, so we can find a time that works well. Thanks!

[lwasser]

thank you @jlpalomino @choldgraf as you go through the review process, will you please submit PR's to update docs as you find issues? thank you in advance!!!

[npch]

@lwasser - yes, still happy to review this.

[lwasser]

wonderful. thank you @npch !! i'll add you as a formal reviewer now! All please be sure to use the reviewer template when you submit your review!!

[choldgraf]

FYI @jlpalomino and I are planning to submit a review early the week after next (I'm outta town for a few days this/next week)

[npch]

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

• A statement of need clearly stating problems the software is designed to solve and its target audience in README

No - the README could be clearer on the target audience for erdapy (e.g. who are the types of researchers most likely to use ERDDAP) and what ERDDAP is (link to ERDDAP info and summarise what it's used for). Much of this information is on the erddapy website, so perhaps summarise and then link?

• Installation instructions: for the development version of package and any non-standard dependencies in README

No - I expected to see short explicit installation instructions, i.e. python setup.py install --user or pip install erddapy (to install from PyPI). Also, see later for problematic dependencies.

• Vignette(s) demonstrating major functionality that runs successfully locally

Partially - there is a good example in the README, but I would have liked to have had it also show what the expected outputs would be (e.g. by showing the first five rows of the data frame produced).

This kind of information is on the website but it isn't linked directly from the README, and I think it's important in the README to show the expected outputs of the example, so the example should go as far as showing a sensible output.

• Function Documentation: for all user-facing functions

Unclear - documentation appears to be being built, but I can't find it (this might be a problem on my system).

• Examples for all user-facing functions

They are on the website, but should be better linked from the README.

• Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer.

No -submitting bug reports are covered but it is not clear how others could (or should) contribute to erddapy. This could just be a simple "get in touch via email" or "submit a pull request".

For packages co-submitting to JOSS

• The package has an obvious research application according to JOSS's definition in their submission requirements.

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

• A short summary describing the high-level functionality of the software
• Authors: A list of authors with their affiliations
• A statement of need clearly stating problems the software is designed to solve and its target audience.
• References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

• Installation: Installation succeeds as documented.

Unable to confirm as it's not documented. However appears to work using python setup.py install --user but not pip install erddapy.

• Functionality: Any functional claims of the software been confirmed.

Yes, able to connect to the test ERDDAP server and retrieve information.

• Performance: Any performance claims of the software been confirmed.

Not applicable.

• Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.

Yes, but please note the following couple of things that need to be addressed:

• Tests required installation of iris, but this seems to be an ambiguous requirement. See: https://pypi.org/project/iris/

• pytzdata is required for tests, but was not in the requirements-dev.txt.

• Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.

Yes - building on Travis CI.

• Packaging guidelines: The package conforms to the pyOpenSci [packaging guidelines]((../packaging/packaging_guide).

Summary of things missing to conform to pyOpenSci packaging guidelines:

• Explicit links to further documentation in README [good]
• Installation instructions [good]
• Links to vignettes in documentation [better/best]
• Improved demonstration usage [better/best]
• Citation information (there as badge, but could be explicit in readme) [better/best]

Final approval (post-review)

• The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing:

• First pass on review: 2 hours

---

Review Comments

See inline comments above.

This package looks pretty close to a great pyOpenSci submission - a few places where there is information to be linked, but is available elsewhere, and some things to be checked and corrected.

[choldgraf]

Just a quite note here that I had a bit of a hard time finding the link to the reviewer template...we should make sure that links for reviewers to use are clearly highlighted in the templates for starting the review process etc!

[lwasser]

hey @npch thank you for your review!!
@choldgraf NOTED...
What do you guys think about creating a thread here: https://pyopensci.discourse.group/c/review-process for feedback on the review process? that way everyone can see it??

@choldgraf where would you like to see the review template starting the process?
Also just checking in on your / @jlpalomino review? is that coming along?

Thanks all!!
@ocefpaf please note Neils comments above re the review !! The next step will be to address the comments

[jlpalomino]

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

• A statement of need clearly stating problems the software is designed to solve and its target audience in README

  • Both the README and the docs should be expanded to provide more details for users new to ERDDAP. The README can be improved with additional details on ERDDAP such as the full name for ERDDAP and a brief description of what it is for unfamiliar users. This information could be copied from the docs (https://ioos.github.io/erddapy/), which contains a great first sentence briefly introducing ERDDAP. The docs can be improved with an explicit hyperlink to the ERDDAP site from the text "Environmental Research Division’s Data Access Program (ERDDAP)".

  • ISSUE: Add details on ERDDAP to README

    • In the README, the ERDDAP acronym should be spelled out as "Environmental Research Division’s Data Access Program (ERDDAP)" and should have an explicit hyperlink to ERDDAP site. In addition, the README should contain a brief description of ERDDAP and an explicit link to the docs for the users that want to learn more information about both ERDDAP and ERDDAPY (similar to first statement provided on docs landing page).

  • ISSUE: Add hyperlink for ERDDAP to docs

    • On the docs landing page, the text "Environmental Research Division’s Data Access Program (ERDDAP)" in the docs should also be expliclty hyperlinked to ERDDAP site (https://coastwatch.pfeg.noaa.gov/erddap/index.html).

• Installation instructions: for the development version of package and any non-standard dependencies in README

  • No installation instructions for package are provided. jlpalomino successfully installed package with pip using pip install erddapy and was able to run the example in README (i.e. connect to ERDDAP server and get data). In addition, we were able to set up the development environment using the packages listed in requirements-dev.txt, but had to add pytz which is not listed in requirements-dev.txt. Chris had difficulty properly installing the environment needed to run erddapy.

  • ISSUE: Add installation instructions to README and docs

    • There are no installation instructions for erddapy in the docs or in the README. A short section in the README and docs called "Installation" with the proper pip install command would be helpful as well as a command for installing in development mode. It would also be helpful to provide the necessary configuration files to re-create the environment for erddapy in Binder.

  • ISSUE: Add pytz to requirements-dev.txt.

    • pytz needs to be added to requirements-dev.txt for tests to run successfully

• Vignette(s) demonstrating major functionality that runs successfully locally

  • The README contains a good start of an example and it runs successfully locally. The example could be improved with comments explaining the code/workflow, some description of the purpose of the example (e.g. the kind of data that it is querying), and displaying the output of the code example. In addition, an example could be provided to show how to get list of the shortcuts available servers (as shown in the docs) and a brief description of what each one contains.

  • The docs contain more detailed vignettes but they are not easy to find, as most are located in the Longer Introduction section of the docs, which is quite long. The docs could be improved by renaming the Quick Introduction to Quick Start and by breaking up the various code sections in the Longer Introduction into separate vignettes with specific titles, similar to the existing examples like OPeNDAP response.

  • ISSUE: Expand example code in README

    • The existing example code in README can be improved with:
      • comments explaining code/workflow
      • brief description of purpose of the example (e.g. the kind of data that it is querying)
      • display of the output of the code
    • Add a new code example to show how to get list of the shortcuts available servers (as shown in the docs) and a brief description of what each one contains.

  • ISSUE: Rename and restructure Quick Introduction and Longer Introduction in the docs

    • The docs can be improved by renaming Quick Introduction to Quick Start and by breaking up the various code sections in the Longer Introduction into separate vignettes with specific titles, similar to the existing examples like OPeNDAP response.

• Function Documentation: for all user-facing functions

  • User-facing functions contain appropriate docstrings in erddapy.py, though utilities.py is less documented (note that many functions in utilities.py are not user-facing). The docs for erddapy (https://ioos.github.io/erddapy/) also provide solid documentation for more functions, with the exception of the to_ functions which have minimal information provided other than links to source code. At a minimum, it would be helpful to provide a list of keyword arguments for each.

  • Finding the link to erddapy functionality in the docs was not obvious, as it was listed as erddapy above Quick introduction in the table of contents and appeared as a subtitle with code formatting, rather than a link (https://ioos.github.io/erddapy/erddapy.html).

  • ISSUE: Add docstrings to user-facing functions in utilities.py

    • User-facing functions in utilities.py should contain apprpriopriate docstrings (e.g. parse_dates).

  • ISSUE: Expand documentation for to_ functions in erddapy

    • The documentation for to_iris, to_pandas, and to_xarray should be expanded to contain a list of keyword arguments for each (e.g. rather than "Accepts any iris.load_raw keyword arguments.")

  • ISSUE: Rename and remove code formatting for erddapy in docs table of contents

    • erddapy should have code formatting removed and be renamed to API Documentation (or something similarly descriptive) to identify it as the link to the functionality documentation (https://ioos.github.io/erddapy/erddapy.html)

• Examples for all user-facing functions

  • The class definition in erddapy.py contains an example, but the function docstrings do not. There are good examples that could be mirrored from the README and the docs.

  • ISSUE: Add examples to docstrings for user-facing functions

    • The class definition in erddapy.py contains an example, but the function docstrings do not. Code examples can be mirrored from the existing examples in the README and the docs.

• Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer.

  • Currently, there is only a link to the issues pages to submit bugs, etc. Package maintainer should be explicit about whether they are interested in contributions. If package maintainer is open to contributions, then the README or CONTRIBUTING should include contribution guidelines.

  • ISSUE: Add explicit statement about contributions to README

    • README should contain an explicit statement about whether package maintainer is interested in contributions. If package maintainer is open to contributions, then the README or CONTRIBUTING needs to be modified to include contribution guidelines.

For packages co-submitting to JOSS

• The package has an obvious research application according to JOSS's definition in their submission requirements.

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

• A short summary describing the high-level functionality of the software
• Authors: A list of authors with their affiliations
• A statement of need clearly stating problems the software is designed to solve and its target audience.
• References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

• Installation: Installation succeeds as documented.

  • Response

    • Note that we could not find any installation instructions for the package. We tried installing with pip using pip install erddapy and this did work. In addition, the requirements needed to run the examples are often difficult to install (for example, cartopy is fairly difficult to install using pip).
    • to run example code and tests, jlpalomino was able to install erddapy, pendulum, pytz, and xarray in a conda environment as defined in https://github.com/earthlab/earth-analytics-python-env/blob/master/environment.yml

  • ISSUE: Add installation instructions

    • There are no installation instructions for erddapy in the documentation or in the README. A quick section called "Installation" that had the proper pip install command would be helpful, as well as a command for installing in development mode.
    • In addition, some packages are fairly difficult to install with pip (specifically, cartopy and iris, any non-python geoscience package such as gdal which is a dependency for many geospatial Python packages). There should be instructions that will work "out of the box" on multiple platforms (or whichever are listed as supported).

• Functionality: Any functional claims of the software been confirmed.

  • Yes - jlpalomino was able to connect to erddapy servers and retrieve data using the example provided in README. Chris wasn't able to get the environment properly-installed, so couldn't confirm that the functionality of the package worked as advertised.

• [ ] Performance: Any performance claims of the software been confirmed.

  • N/A

• Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.

  • In general, the tests look to be in good shape with one hiccup. We tried running the tests locally and ran into a little problem - pytz is needed but not listed in requirements-dev.txt. After that, tests ran successfully locally.

  • ISSUE: Add pytz to requirements-dev.txt

• Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.

  • Travis seems to be running the test suite and the badge shows that all looks well. Code coverage seems to be configured, but it's unclear where the coverage reports exist. This should be documented as well.

  • ISSUE: Add a badge for code coverage in README and doc

    • Currently, there isn't any indication of the code coverage, which makes it hard to gauge the effectiveness of the tests. It would be great to use a service such as codecov and display as a badge in the readme/docs.

• Packaging guidelines: The package conforms to the pyOpenSci [packaging guidelines]((../packaging/packaging_guide).

  • There's no extra stuff to cover here that hasn't been covered above.

Final approval (post-review)

• The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 2-3 for each reviewer

---

Review Comments

erddapy is a well-scoped tool that has a lot of great functionality. The documentation for the tool has good content, but could be improved with some re-structuring to make content easier to find and follow. There were some basic pieces of information missing that should be added (such as an "installation" section) and standardized across the README and the documentation for the package.

[lwasser]

hey @ocefpaf just checking in with you again to see if you've seen the review feedback here. Please get in touch with any questions and with a timeline for when you think you can address the review comments. Hope you are well!

[ocefpaf]

hey @ocefpaf just checking in with you again to see if you've seen the review feedback here. Please get in touch with any questions and with a timeline for when you think you can address the review comments. Hope you are well!

Will do soon. Just swamped with the day job after 15 days away. (I should be able to get back to this later today/early tomorrow.)

[lwasser]

ok wonderful. thank you @ocefpaf i hope your 15 days away were fun rather than work!! i understanding the catch up game. Please get in touch if you need anything to begin to respond to the reviewers or from pyopensci in general!

[ocefpaf]

I hope your 15 days away were fun rather than work!

Both actually. It was fun work.

Here are the answers for the review: #1 (comment). I'll respond to #1 (comment) later in separately.

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

• A statement of need clearly stating problems the software is designed to solve and its target audience in README

  • Both the README and the docs should be expanded to provide more details for users new to ERDDAP. The README can be improved with additional details on ERDDAP such as the full name for ERDDAP and a brief description of what it is for unfamiliar users. This information could be copied from the docs (ioos.github.io/erddapy), which contains a great first sentence briefly introducing ERDDAP. The docs can be improved with an explicit hyperlink to the ERDDAP site from the text "Environmental Research Division’s Data Access Program (ERDDAP)".
  • ISSUE: Add details on ERDDAP to README
    • In the README, the ERDDAP acronym should be spelled out as "Environmental Research Division’s Data Access Program (ERDDAP)" and should have an explicit hyperlink to ERDDAP site. In addition, the README should contain a brief description of ERDDAP and an explicit link to the docs for the users that want to learn more information about both ERDDAP and ERDDAPY (similar to first statement provided on docs landing page).

There is not a full, ERDDAP is not an acronym. It is just ERDDAP.
Regarding the details I believe that this part of the README has what you are
saying (same info from the docs that you reference):

Easier access to scientific data.

erddapy takes advantage of ERDDAP's RESTful web services and creates the ERDDAP URL for any request, like searching for datasets, acquiring metadata, downloading the data, etc.

What is ERDDAP? ERDDAP unifies the different types of data servers and offers a consistent way to get the data in multiple the formats.

Not sure how that can be improved without sending users to ERDDAP's documentation.
(Link added in ioos/erddapy#78)

BTW I don't think users will find erddapy without knowing what ERDDAP is.
It is a wrapper package and the nature of wrapper packages is that
they are tied to the hip with the package they are wrapping.

• ISSUE: Add hyperlink for ERDDAP to docs
  • On the docs landing page, the text "Environmental Research Division’s Data Access Program (ERDDAP)" in the docs should also be expliclty hyperlinked to ERDDAP site (coastwatch.pfeg.noaa.gov/erddap/index.html).

I guess the reviewer was confused by the information about ERDDAP online.
The one that the reviewer found is wrong and/or outdated.
Here is what the author has to say about the name:

What does the acronym "ERDDAP" stand for? 
"ERDDAP" used to be an acronym, but it outgrew that original description. Now, please just think of it as a name, not an acronym. 

The correct link is already in the docs.
See The link is in: https://github.com/ioos/erddapy/blob/master/docs/source/index.rst#L31

• Installation instructions: for the development version of package and any non-standard dependencies in README

  • No installation instructions for package are provided. jlpalomino successfully installed package with pip using pip install erddapy and was able to run the example in README (i.e. connect to ERDDAP server and get data). In addition, we were able to set up the development environment using the packages listed in requirements-dev.txt, but had to add pytz which is not listed in requirements-dev.txt. Chris had difficulty properly installing the environment needed to run erddapy.

  • ISSUE: Add installation instructions to README and docs

    • There are no installation instructions for erddapy in the docs or in the README. A short section in the README and docs called "Installation" with the proper pip install command would be helpful as well as a command for installing in development mode. It would also be helpful to provide the necessary configuration files to re-create the environment for erddapy in Binder.

Fixed in ioos/erddapy#79.

• ISSUE: Add pytz to requirements-dev.txt.

  • pytz needs to be added to requirements-dev.txt for tests to run successfully

pytz is a dependency of a dependency (iris), no need to add it there.
I guess that the issue was with the pip installation of iris.

• Vignette(s) demonstrating major functionality that runs successfully locally

  • The README contains a good start of an example and it runs successfully locally. The example could be improved with comments explaining the code/workflow, some description of the purpose of the example (e.g. the kind of data that it is querying), and displaying the output of the code example. In addition, an example could be provided to show how to get list of the shortcuts available servers (as shown in the docs) and a brief description of what each one contains.

  • The docs contain more detailed vignettes but they are not easy to find, as most are located in the Longer Introduction section of the docs, which is quite long. The docs could be improved by renaming the Quick Introduction to Quick Start and by breaking up the various code sections in the Longer Introduction into separate vignettes with specific titles, similar to the existing examples like OPeNDAP response.

  • ISSUE: Expand example code in README

    • The existing example code in README can be improved with:

      • comments explaining code/workflow
      • brief description of purpose of the example (e.g. the kind of data that it is querying)
      • display of the output of the code

    • Add a new code example to show how to get list of the shortcuts available servers (as shown in the docs) and a brief description of what each one contains.

I do not grasp the concept of Vignette in README and I do not want to make the README more verbose. We have docs for that. I can expand more on this in a call but, IMO, READMEs should have a very quick intro/example or no example at all, and links and references to the docs.

• ISSUE: Rename and restructure Quick Introduction and Longer Introduction in the docs

  • The docs can be improved by renaming Quick Introduction to Quick Start and by breaking up the various code sections in the Longer Introduction into separate vignettes with specific titles, similar to the existing examples like OPeNDAP response.

It is not clear to me how that would improve the docs.
It is meant to be a single workflow, that users of ERDDAP usually do,
and separating into sections may actually make it confusing.

The OPeNDAP repose is separate b/c it does not belong in that workflow.

• Function Documentation: for all user-facing functions

  • User-facing functions contain appropriate docstrings in erddapy.py, though utilities.py is less documented (note that many functions in utilities.py are not user-facing). The docs for erddapy (ioos.github.io/erddapy) also provide solid documentation for more functions, with the exception of the to_ functions which have minimal information provided other than links to source code. At a minimum, it would be helpful to provide a list of keyword arguments for each.

  • Finding the link to erddapy functionality in the docs was not obvious, as it was listed as erddapy above Quick introduction in the table of contents and appeared as a subtitle with code formatting, rather than a link (ioos.github.io/erddapy/erddapy.html).

  • ISSUE: Add docstrings to user-facing functions in utilities.py

    • User-facing functions in utilities.py should contain apprpriopriate docstrings (e.g. parse_dates).

All users facing functions in utilities.py have docstrings. Also, the docs you mention are autogenerated from those docstrings. I'm confused with your comment above.

• ISSUE: Expand documentation for to_ functions in erddapy

  • The documentation for to_iris, to_pandas, and to_xarray should be expanded to contain a list of keyword arguments for each (e.g. rather than "Accepts any iris.load_raw keyword arguments.")

Thanks for the suggestion but that does not belong here. The docstrings mention the upstream documentation and copying that here will make the to_ functions unnecessarily verbose.

• ISSUE: Rename and remove code formatting for erddapy in docs table of contents

  • erddapy should have code formatting removed and be renamed to API Documentation (or something similarly descriptive) to identify it as the link to the functionality documentation (ioos.github.io/erddapy/erddapy.html)

I do not follow this comment. That is the documentation for the erddapy.py module and has the right name. Also, the code formatting there standard for python documentation and the theme used.

• Examples for all user-facing functions

  • The class definition in erddapy.py contains an example, but the function docstrings do not. There are good examples that could be mirrored from the README and the docs.

  • ISSUE: Add examples to docstrings for user-facing functions

    • The class definition in erddapy.py contains an example, but the function docstrings do not. Code examples can be mirrored from the existing examples in the README and the docs.

I don't see the point of that. Functions docstrings should be concise, classes and docs with examples. The Python machinery for investigating the docs online or interactive take advantage of that and repeating information can bloat the code base with stale and out of sync examples.

• Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer.

  • Currently, there is only a link to the issues pages to submit bugs, etc. Package maintainer should be explicit about whether they are interested in contributions. If package maintainer is open to contributions, then the README or CONTRIBUTING should include contribution guidelines.

  • ISSUE: Add explicit statement about contributions to README

    • README should contain an explicit statement about whether package maintainer is interested in contributions. If package maintainer is open to contributions, then the README or CONTRIBUTING needs to be modified to include contribution guidelines.

The code is OSS and the code if hosted on GitHub, so I don't think we should explicitly say we are open to contributions when that is obvious IMO.
Regarding a CONTRIBUTING file, NOAA has a policy on that and I'll check with them how we can added it here.

Functionality

• Installation: Installation succeeds as documented.

  • Response

    • Note that we could not find any installation instructions for the package. We tried installing with pip using pip install erddapy and this did work. In addition, the requirements needed to run the examples are often difficult to install (for example, cartopy is fairly difficult to install using pip).
    • to run example code and tests, jlpalomino was able to install erddapy, pendulum, pytz, and xarray in a conda environment as defined in earthlab/earth-analytics-python-env:environment.yml@master

  • ISSUE: Add installation instructions

    • There are no installation instructions for erddapy in the documentation or in the README. A quick section called "Installation" that had the proper pip install command would be helpful, as well as a command for installing in development mode.
    • In addition, some packages are fairly difficult to install with pip (specifically, cartopy and iris, any non-python geoscience package such as gdal which is a dependency for many geospatial Python packages). There should be instructions that will work "out of the box" on multiple platforms (or whichever are listed as supported).

Fixed in ioos/erddapy#79.
I do not want to add a development more installation b/c devs usually have different opinions on how to do that and I prefer to let them use the tools they are more comfortable with.

The reviewer had some trouble with the iris name being different on PyPI vs the original package name. I'll add a note on that but, b/c that is development, I will not add in the README. (The iris authors are trying to get the name on PyPI and that problem should go away soon.)

Regarding the platforms supported. I think that is beyond the scope of this library and any possible dev guide we add. All those libraries are pip and conda install-able with different degrees of difficult and devs choose their tools based on their preferences. Adding such guides we are responsible for maintaining workflows that may go stable quickly. Ideally we should let the best practices for package dev installation out of the libraries.

• Functionality: Any functional claims of the software been confirmed.

  • Yes - jlpalomino was able to connect to erddapy servers and retrieve data using the example provided in README. Chris wasn't able to get the environment properly-installed, so couldn't confirm that the functionality of the package worked as advertised.

• [ ] Performance: Any performance claims of the software been confirmed.

  • N/A

• Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.

  • In general, the tests look to be in good shape with one hiccup. We tried running the tests locally and ran into a little problem - pytz is needed but not listed in requirements-dev.txt. After that, tests ran successfully locally.
  • ISSUE: Add pytz to requirements-dev.txt

Should be addressed with the iris name disambiguation.
There is no need to add it.
See ioos/erddapy#80

• Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.

  • Travis seems to be running the test suite and the badge shows that all looks well. Code coverage seems to be configured, but it's unclear where the coverage reports exist. This should be documented as well.

  • ISSUE: Add a badge for code coverage in README and doc

    • Currently, there isn't any indication of the code coverage, which makes it hard to gauge the effectiveness of the tests. It would be great to use a service such as codecov and display as a badge in the readme/docs.

Is that mandatory for a PyOpenSci project? If not I prefer to not add it.
Code coverage can be helpful for a developer locally but it is a bogus code quality in general. It should be used only by the developers to gauge where the flaws in the docs are, otherwise it is easy to fake a 100% code coverage and pretend the tests are in good health.

• Packaging guidelines: The package conforms to the pyOpenSci [packaging guidelines]((../packaging/packaging_guide).

  • There's no extra stuff to cover here that hasn't been covered above.

Thanks to the reviewers. This was an interesting exercise. I believe I have to read the review guidelines and I would love to get some feedback later. Hope to see you all in the next PyOpenSci call!

[ocefpaf]

Responses for review #1 (comment):

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

• A statement of need clearly stating problems the software is designed to solve and its target audience in README

No - the README could be clearer on the target audience for erdapy (e.g. who are the types of researchers most likely to use ERDDAP) and what ERDDAP is (link to ERDDAP info and summarise what it's used for). Much of this information is on the erddapy website, so perhaps summarise and then link?

I believe the present summary is enough.
The nature of wrapper libraries is that the target users already know the library being wrapped. If that is not true there is a link to the ERDDAP page to send them to the docs.

• Installation instructions: for the development version of package and any non-standard dependencies in README

No - I expected to see short explicit installation instructions, i.e. python setup.py install --user or pip install erddapy (to install from PyPI). Also, see later for problematic dependencies.

Done. The PRs fixing that are referenced in the other review.

• Vignette(s) demonstrating major functionality that runs successfully locally

Partially - there is a good example in the README, but I would have liked to have had it also show what the expected outputs would be (e.g. by showing the first five rows of the data frame produced).

This kind of information is on the website but it isn't linked directly from the README, and I think it's important in the README to show the expected outputs of the example, so the example should go as far as showing a sensible output.

I disagree with that. It will bloat the README and it is beyond the scope. That is why we have the docs. Also, the expected output varies a lot depending on the data queried, so there is no expected output but only an expected type.

• Function Documentation: for all user-facing functions

Unclear - documentation appears to be being built, but I can't find it (this might be a problem on my system).

• Examples for all user-facing functions

They are on the website, but should be better linked from the README.

All functions have docstrings and the docs are build with sphinx in the doc page mention.
They do not belong in the README.

• Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer.

No -submitting bug reports are covered but it is not clear how others could (or should) contribute to erddapy. This could just be a simple "get in touch via email" or "submit a pull request".

Does PyOpenSci have templates for this? I'll check later and add some but I need to clear with NOAA first b/c they have some policies on this.

Functionality

• Installation: Installation succeeds as documented.

Unable to confirm as it's not documented. However appears to work using python setup.py install --user but not pip install erddapy.

Added a note about this for pip users in ioos/erddapy#80

• Functionality: Any functional claims of the software been confirmed.

Yes, able to connect to the test ERDDAP server and retrieve information.

• Performance: Any performance claims of the software been confirmed.

Not applicable.

• Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.

Yes, but please note the following couple of things that need to be addressed:

• Tests required installation of iris, but this seems to be an ambiguous requirement. See: pypi.org/project/iris
• pytzdata is required for tests, but was not in the requirements-dev.txt.

Also addressed in ioos/erddapy#80 both have to do with iris name on PyPI.

• Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.

Yes - building on Travis CI.

• Packaging guidelines: The package conforms to the pyOpenSci [packaging guidelines]((../packaging/packaging_guide).

Summary of things missing to conform to pyOpenSci packaging guidelines:

• Explicit links to further documentation in README [good]
• Installation instructions [good]
• Links to vignettes in documentation [better/best]
• Improved demonstration usage [better/best]
• Citation information (there as badge, but could be explicit in readme) [better/best]

[lwasser]

thank you @ocefpaf for the detailed responses to the reviewers @npch , @choldgraf and @jlpalomino
I will have a look at this in more detail in a bit. it looks like we have some questions with respect to

1. expectations how we document functions, etc
2. expectations about contributing and associated documentation
3. documentation in general - what should go in a readme vs actual docs . (filipe - i'm with you in the "keep the readme simple" comments but do believe in well crafted docs) let's talk more!!

More in a bit - i just wanted to acknowledge and thank you all for the effort put into this review so far.

[lwasser]

ok working through all of the wonderful content in these reviews and this package!! I am going to start with @npch because it brought up some really good questions that i also have. note - this is our first review so thank you BOTH for your patience. I suspect we may move a few discussion points over to discourse.

@ocefpaf it looks like you have addressed a many things already including the installation instructions! A few questions i have.

• as someone who isn't familiar with ERDDAP I am not sure what data it can access. Thus it could be super useful to me as i work with different types of data, but there is no statement that clarifies what ERDDAP is. Imagine ERDDAPY in our list of pyopensci packages. I'd love for someone to better understand what exactly this one does even if they aren't familiar with ERDDAPY. Words like gridded and tabular and noaa and maybe coastal (are all of the data coastal and are they global or US focused??). this would greatly clarify and you could add this all in one sentence i think or maybe two. Could you consider a statement in the readme like (please do edit as you see fit):

 ERDDAPY makes it easier to access NOAA gridded and tabular scientific datasets in common file formats from Python. 

erddapy takes advantage of ERDDAP's RESTful web services and creates the ERDDAP URL for any request, like searching for datasets, acquiring metadata, downloading the data, etc.

What is ERDDAP? ERDDAP unifies the different types of data servers and offers a consistent way to get the data in multiple the formats. For more information on ERDDAP servers please see https://coastwatch.pfeg.noaa.gov/erddap/index.html

Here are my two cents: i think users will find edrappy now via pyopensci. and you'll get new users but only if they know what it does! :) and gosh it's close - just a few more clarifying words will go a long way.

• i see your documentation is there with autodoc enabled! it's great. but how do i find it? when i go to the docs, i don't see it on the side bar.

i am used to seeing API documentation on the side bar like this on the sidebar. it would be great if something like that existed in the docs as i wasn't sure how to find it either but did see the effort put into docstring code. This would make it easier for users to find the documentation which has great examples. To address neil's comment i wonder if you could also link to the "full api documentation" in the readme below the getting started example. just a thought Filipe.

• Dev setup -- iris
  @ocefpaf just a question here. i'm used to a suggestion of
  pip install requirements-dev.txt so providing one working example of dev setup but then if people want to use conda or whatever else let them do that. i'm just curious if you could consider something like that to address the iris issue or if a user will ALWAYS have to edit the dev-requirements file based on how they install? it is a bit confusing to me but gosh so is the entire world of channels and package versions and such as things get mixed constantly it seems :) or i'm always battling it anyway.

it looks like your default is conda conda create --name TEST python=$PY --file requirements.txt --file requirements-dev.txt so maybe you could just provide those instructions and tell people to modify the file if they use something other than conda?

• quick start vs short introduction -- my two cents again - https://ioos.github.io/erddapy/quick_intro.html# i am used to seeing words like quickstart for code vignettes that get things started. when i read introduction i think i'm going to get text about your package. should we have a quick discussion in discourse about this to see what others think? i think the comments are just getting as usability. I do think the usability could be improved a bit ... if you are up for it and if others agree! but not something to hang up a review around. with that said if you named the vignettes in your docs quick start or vignette or something you could just provide a link in the readme and be done with it! then things would be clear.

• @npch i'm also confused by this statement

erddapy should have code formatting removed and be renamed to API Documentation (or something similarly descriptive) to identify it as the link to the functionality documentation (ioos.github.io/erddapy/erddapy.html)

but maybe i follow... would adding a link to API DOCUMENTATION in the readme and to the docs left side panel address the issue? if not can you kindly clarify?

• Community guidelines comment
  I'd like to make a suggestion. i do understand that OSS on github should by default welcome contributions BUT... let's not assume people understand that is the case! i've seen SO many scientists and students who don't understand this concept. I'd like to see ALL of our pyopensci projects have clear contribution guidelines (whatever they are!). @ocefpaf are you open to that? i hear your point but think it's good to be explicit where we can. and we can be concise about it too.

The code is OSS and the code if hosted on GitHub, so I don't think we should explicitly say we are open to contributions when that is obvious IMO.
Regarding a CONTRIBUTING file, NOAA has a policy on that and I'll check with them how we can added it here.

ok...i'll look at the next review later but i wanted to address this first review first. @ocefpaf can you kindly see my above comments and tell me if they are things you're willing to consider ? i'm trying to find a middle ground between the reviews and pyopensci goals. But also there are a few topics that we probably should discuss more outside of this review!!

Also as you respond, please note our readme guidelines as we can change things if we need to! i think we could add this checklist to the review template too. would that be useful @npch @choldgraf (not pinging jenny as she is out for two weeks!!)

README
All packages should have a README.md in their root directory. The README should include, from top to bottom:

The package name
Badges for continuous integration and test coverage, the badge for pyOpenSci peer-review once it has started (see below), a repostatus.org badge, and any other badges. If the README has many more badges, you might want to consider using a table for badges, see this example, that one and that one. Such a table should be more wide than high.
Short description of goals of package, with descriptive links to all vignettes (rendered, i.e. readable, cf the documentation website section) unless the package is small and there’s only one vignette repeating the README.
Installation instructions
Any additional setup required (authentication tokens, etc)
Brief demonstration usage
Direction to more detailed documentation (e.g. your documentation files or website).
If applicable, how the package compares to other similar packages and/or how it relates to other packages
Citation information

[ocefpaf]

• as someone who isn't familiar with ERDDAP I am not sure what data it can access. Thus it could be super useful to me as i work with different types of data, but there is no statement that clarifies what ERDDAP is. Imagine ERDDAPY in our list of pyopensci packages. I'd love for someone to better understand what exactly this one does even if they aren't familiar with ERDDAPY. Words like gridded and tabular and noaa and maybe coastal (are all of the data coastal and are they global or US focused??). this would greatly clarify and you could add this all in one sentence i think or maybe two. Could you consider a statement in the readme like (please do edit as you see fit):

 ERDDAPY makes it easier to access NOAA gridded and tabular scientific datasets in common file formats from Python. 

erddapy takes advantage of ERDDAP's RESTful web services and creates the ERDDAP URL for any request, like searching for datasets, acquiring metadata, downloading the data, etc.

What is ERDDAP? ERDDAP unifies the different types of data servers and offers a consistent way to get the data in multiple the formats. For more information on ERDDAP servers please see https://coastwatch.pfeg.noaa.gov/erddap/index.html

Here are my two cents: i think users will find edrappy now via pyopensci. and you'll get new users but only if they know what it does! :) and gosh it's close - just a few more clarifying words will go a long way.

The problem is that ERDDAP is a generic scientific data server and any attempt of describing the datesets in it will come short. The description you got above is of one ERDDAP instance.
I could create another one with sound data from whales and then the description would be wrong. That is why I prefer the generic (and I know vague) "Easier access to scientific data." I asked the author of ERDDAP about that and he agrees that describing the type of datasets are not desirable but he recommends to add something about the various data sources/outputs though (csv, netCDF, hdf, JSON, wav, etc).

• i see your documentation is there with autodoc enabled! it's great. but how do i find it? when i go to the docs, i don't see it on the side bar.

i am used to seeing API documentation on the side bar like this on the sidebar. it would be great if something like that existed in the docs as i wasn't sure how to find it either but did see the effort put into docstring code. This would make it easier for users to find the documentation which has great examples. To address neil's comment i wonder if you could also link to the "full api documentation" in the readme below the getting started example. just a thought Filipe.

The theme I'm using does not add it to the side-bar. It is here:

• Dev setup -- iris
  @ocefpaf just a question here. i'm used to a suggestion of
  pip install requirements-dev.txt so providing one working example of dev setup but then if people want to use conda or whatever else let them do that. i'm just curious if you could consider something like that to address the iris issue or if a user will ALWAYS have to edit the dev-requirements file based on how they install? it is a bit confusing to me but gosh so is the entire world of channels and package versions and such as things get mixed constantly it seems :) or i'm always battling it anyway.

It is a development dependency, so user are not forced to use anything besides pip if they do not want to. Also, even as a development dependency, unless someone is dealing with the to_iris method they do not need iris installed. I have it there just to test that functionality that method.

Last but not least I will answer with this issue: SciTools/iris#3321

Pretty much iris is impossible to install with pip!

it looks like your default is conda conda create --name TEST python=$PY --file requirements.txt --file requirements-dev.txt so maybe you could just provide those instructions and tell people to modify the file if they use something other than conda?

That is not necessary for uses. I'm looking into it for a development guide.

• quick start vs short introduction -- my two cents again - ioos.github.io/erddapy/quick_intro.html# i am used to seeing words like quickstart for code vignettes that get things started. when i read introduction i think i'm going to get text about your package. should we have a quick discussion in discourse about this to see what others think? i think the comments are just getting as usability. I do think the usability could be improved a bit ... if you are up for it and if others agree! but not something to hang up a review around. with that said if you named the vignettes in your docs quick start or vignette or something you could just provide a link in the readme and be done with it! then things would be clear.

If PyOpenSci has strong opinions about that I can change but I can't really see how the name of the section can be confusing to the understanding of the docs.

• @npch i'm also confused by this statement

erddapy should have code formatting removed and be renamed to API Documentation (or something similarly descriptive) to identify it as the link to the functionality documentation (ioos.github.io/erddapy/erddapy.html)

but maybe i follow... would adding a link to API DOCUMENTATION in the readme and to the docs left side panel address the issue? if not can you kindly clarify?

• Community guidelines comment
  I'd like to make a suggestion. i do understand that OSS on github should by default welcome contributions BUT... let's not assume people understand that is the case! i've seen SO many scientists and students who don't understand this concept. I'd like to see ALL of our pyopensci projects have clear contribution guidelines (whatever they are!). @ocefpaf are you open to that? i hear your point but think it's good to be explicit where we can. and we can be concise about it too.

I guess that as long as PyOpenSci provides a template for those I do not have a problem requiring them for package submissions. What I would like to avoid is: you are missing a CONTRIBUTING.md please add one. And then, let's review your CONTRIBUTING.md...

The code is OSS and the code if hosted on GitHub, so I don't think we should explicitly say we are open to contributions when that is obvious IMO.
Regarding a CONTRIBUTING file, NOAA has a policy on that and I'll check with them how we can added it here.

ok...i'll look at the next review later but i wanted to address this first review first. @ocefpaf can you kindly see my above comments and tell me if they are things you're willing to consider ? i'm trying to find a middle ground between the reviews and pyopensci goals. But also there are a few topics that we probably should discuss more outside of this review!!

Also as you respond, please note our readme guidelines as we can change things if we need to! i think we could add this checklist to the review template too. would that be useful @npch @choldgraf (not pinging jenny as she is out for two weeks!!)

I think the README below is OK.

README
All packages should have a README.md in their root directory. The README should include, from top to bottom:

The package name
Badges for continuous integration and test coverage, the badge for pyOpenSci peer-review once it has started (see below), a repostatus.org badge, and any other badges. If the README has many more badges, you might want to consider using a table for badges, see this example, that one and that one. Such a table should be more wide than high.
Short description of goals of package, with descriptive links to all vignettes (rendered, i.e. readable, cf the documentation website section) unless the package is small and there’s only one vignette repeating the README.
Installation instructions
Any additional setup required (authentication tokens, etc)
Brief demonstration usage
Direction to more detailed documentation (e.g. your documentation files or website).
If applicable, how the package compares to other similar packages and/or how it relates to other packages
Citation information

[lwasser]

ok all... I need to get this moving. the reason why i've been stalling is that not all reviewer comments were implemented and i'm not sure what the best way forward is to ensure everyone feels satisfied. @choldgraf @npch @ocefpaf should we have a discussion over in the discourse forum on

1. how much documentation do we require and what types of feedback do we require authors to implement
2. how we create usability requirements in a way that are implementable but not overly intensive to apply
3. what should go in the readme and what we enforce.

it would be good to get this package through the review process i just would like us to make a few decisions on how we make decisions on such things.

[lwasser]

ok all -- we are going to do a reboot of this submission. @ocefpaf today we had a good conversation around package usability and what pyopensci wants to see from packages. There was general agreement that we should move towards packages that are more usable to a larger audience of people. this includes things like clear documentation , readmes, etc etc.

We also agreed that this issue is very large and hard to keep tabs on at this point.

Are you ok with @jlpalomino and @choldgraf revisiting this issue,, opened a second time so we can start fresh with the new review template that allows for issues to be opened and PR's (if you are open to that). we will then have a very organized list of items for you to work on which will align well with the review that you submitted for martin. does that work for you?

[choldgraf]

I can take another pass from a fresh issue and a new template if that's helpful - I must admit that I have been putting off responding on this particular thread because the issue is long and intimidating to go through :-)

[lwasser]

@choldgraf we all agree that this issue has gotten massive. my pr in dev_guide begins to address some of the issues here. pyOpenSci/software-peer-review#31 if you'd like to resubmit this and start fresh, that would be wonderful. Essentially in our meeting we agreed that usability is core to pyopensci that is captured in the new edits to the review template.

[lwasser]

hey team!! it's been a while since we have revisited this review. Let's try again using the new template and via submitting issues / PR's as it makes sense. @choldgraf @jlpalomino are you game to kickstart that process?? thank you in advance!! and thank you @ocefpaf and all for your patience!!

[jlpalomino]

Sounds good @lwasser. I'll coordinate with @choldgraf to finish a new review using the revised template.

[lwasser]

thank you so much @jlpalomino :)

[jlpalomino]

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

• A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • I really like the explanations of what ERDDAP is in the README and documentation - it gives me a better idea for what role the tool plays, and why this package is useful in helping use that tool.
• Installation instructions: for the development version of package and any non-standard dependencies in README
• Vignette(s) demonstrating major functionality that runs successfully locally
• Function Documentation: for all user-facing functions
• Examples for all user-facing functions
• Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer.

Readme requirements
The package meets the readme requirements below:

• Package has a README.md file in the root directory.

The README should include, from top to bottom:

• The package name
• Badges for continuous integration and test coverage, the badge for pyOpenSci peer-review once it has started (see below), a repostatus.org badge, and any other badges. If the README has many more badges, you might want to consider using a table for badges, see this example, that one and that one. Such a table should be more wide than high.
• Short description of goals of package, with descriptive links to all vignettes (rendered, i.e. readable, cf the documentation website section) unless the package is small and there’s only one vignette repeating the README.
• Installation instructions
• Any additional setup required (authentication tokens, etc)
• Brief demonstration usage
• Direction to more detailed documentation (e.g. your documentation files or website).
• If applicable, how the package compares to other similar packages and/or how it relates to other packages
• Citation information

Functionality

• Installation: Installation succeeds as documented.
• Functionality: Any functional claims of the software been confirmed.
• Performance: Any performance claims of the software been confirmed.
• Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
• Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.
• Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.

For packages co-submitting to JOSS

• The package has an obvious research application according to JOSS's definition in their submission requirements.

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

• A short summary describing the high-level functionality of the software
• Authors: A list of authors with their affiliations
• A statement of need clearly stating problems the software is designed to solve and its target audience.
• References: with DOIs for all those that have one (e.g. papers, datasets, software).

Final approval (post-review)

• The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing:
2-3 per reviewer

---

Review Comments

Notes from the reviewers

Review from choldgraf

In general, this package does a good job of explaining the problem
that it is trying to solve, as well as basic usage for how to get
started with the tool. The installation and example code worked
well for me! The main improvements could be relatively minor things
in the documentation, such as adding more links between the README
and the website documentation. I also think that the documentation
examples could be split into more "how-to" sections rather than a
single all-encompassing tutorial; however, I think those can be
improved in the future and shouldn't be a blocker for this review.
In all, it appears to be a well-designed tool!

• I like the list of badges at the top!
• Recommend adding a link to the notebook in the docs #85 - [name=chris h]
• Recommend calling the API page "erddapy API" #86 - [name=chris h]
• Add links to the documentation in the readme #87 - [name=chris h]
• It's hard to know if there's an example for each user-facing function.
  What is a good way we can ask people to check this without "ctrl-f"ing
  through the whole documentation?
• There's information on how to get in touch now, though still not official
  guidelines for how to get involved or contribute. If the answer to that
  is "I'd prefer to be the one writing the code, and don't plan on creating
  a big community around this project", I think that's fine, it should just
  be mentioned explicitly. That said, I don't think it's a blocker.
• There's no citation information, is that OK?
• There's no JOSS-specific configuration (e.g. no paper.md). We should
  confirm that the author isn't interested in submitting this to JOSS.
• Another thing I noticed: we don't have any explicit review items
  about code review, style, etc. Not sure where we decided on that
  point, but just wanted to point it out.

Review from jlpalomino

I agree with my co-reviewer's comments and open issues for this package. erddapy is a well-scoped tool with strong functionality for accessing ERDDAP data. The installation of the package (using both conda and pip) and the short example worked for me. The installation of requirements-dev.txt also worked (using conda), and while not required, it could be a good addition to include instructions for that installation as well, perhaps in contributing guidelines, if the package author is open to that. I agree with my co-reviewer's comments that it should not a considered a block but it may be good to specify to users whether or not the author is interested in contributions, if there will not be contributing guidelines.

The documentation for the tool has strong content, and it would be great to have a link to it directly in the README, as my co-reviewer suggests. The "quick introduction" is solid and easy to follow, though it would be nice to link here again to where the user can find out more information about the servers/datasets that are accessible with erddapy. Similar to my co-reviewer's comments about the documentation, I think the "longer introduction" could be split into smaller how-to" sections to make specific functionality a little easier to follow and easier to find, but it should not be considered a block and can be completed over time. Overall, the tool is fairly well-documented and easy to implement.

[jlpalomino]

@lwasser @ocefpaf Please see above the review for erddapy by choldgraf and myself using the latest template.

[lwasser]

@ocefpaf just a friendly ping to see how if you have time to respond to the latest review . Also please note at the top of this issue I did confirm that Filipe is NOT interested in the JOSS component of the review at this time @jlpalomino

[lwasser]

Good morning, everyone. Because this issue has remained open since 2019 with not much movement, I am going to close it. @choldgraf @jlpalomino @ocefpaf @xmnlab @npch thank you all so much for helping us refine our review process through this review. If in the future Filipe, you wish to resubmit this you are welcome to. In the meantime pyopensci will be running in full force as of today (september 1) now that I am full time on the project. I can't wait to continue working with all of you on this effort.

For now... closing this issue. :)

[ocefpaf]

In the meantime pyopensci will be running in full force as of today (september 1) now that I am full time on the project. I can't wait to continue working with all of you on this effort.

Those are great news! Glad to hear it!!

PS: the erddapy application was never really meant to be a real one, just a test-drive of the workflow. I'd be happy to re-submit a real one as soon as possible,

[lwasser]

@ocefpaf hi!! 👋 yes i'm very excited and even more excited to get to work with you and the conda-forge team as it makes sense :) i totally understand erddapy was intended to be submitted to test our review process and we did learn a lot from it! you are welcome to submit to us any time you wish!! there is no rush - we aren't going anywhere!! Please do feel free to reach out if pyopensci can support any of the work you've been doing as well in the python ecosystem. 🎆