Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: Python for Atmosphere and Ocean Scientists #37

Closed
44 tasks done
whedon opened this issue Nov 21, 2018 · 33 comments
Closed
44 tasks done

[REVIEW]: Python for Atmosphere and Ocean Scientists #37

whedon opened this issue Nov 21, 2018 · 33 comments
Assignees

Comments

@whedon
Copy link

whedon commented Nov 21, 2018

Submitting author: @DamienIrving (Damien Irving)
Repository: https://github.com/carpentrieslab/python-aos-lesson
Version: v1.0.0
Editor: @willingc
Reviewer: @darothen, @RobTheOceanographer
Archive: 10.5281/zenodo.2546005

Status

status

Status badge code:

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

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

@darothen & @RobTheOceanographer, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/jose-reviews/invitations

The reviewer guidelines are available here: https://jose.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @willingc know.

Review checklist for @darothen

Conflict of interest

Code of Conduct

General checks

  • Repository: Is the source for this learning module available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of a standard license? (OSI-approved for code, Creative Commons for content)
  • Version: Does the release version given match the repository release (v1.0.0)?
  • Authorship: Has the submitting author (@DamienIrving) made visible contributions to the module? Does the full list of authors seem appropriate and complete?

Documentation

  • A statement of need: Do the authors clearly state the need for this module and who the target audience is?
  • Installation instructions: Is there a clearly stated list of dependencies?
  • Usage: Does the documentation explain how someone would adopt the module, and include examples of how to use it?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the module 2) Report issues or problems with the module 3) Seek support

Pedagogy / Instructional design (Work-in-progress: reviewers, please comment!)

  • Learning objectives: Does the module make the learning objectives plainly clear? (We don't require explicitly written learning objectives; only that they be evident from content and design.)
  • Content scope and length: Is the content substantial for learning a given topic? Is the length of the module appropriate?
  • Pedagogy: Does the module seem easy to follow? Does it observe guidance on cognitive load? (working memory limits of 7 +/- 2 chunks of information)
  • Content quality: Is the writing of good quality, concise, engaging? Are the code components well crafted? Does the module seem complete?
  • Instructional design: Is the instructional design deliberate and apparent? For example, exploit worked-example effects; effective multi-media use; low extraneous cognitive load.

JOSE paper

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Does the paper clearly state the need for this module and who the target audience is?
  • Description: Does the paper describe the learning materials and sequence?
  • Does it describe how it has been used in the classroom or other settings, and how someone might adopt it?
  • Could someone else teach with this module, given the right expertise?
  • Does the paper tell the "story" of how the authors came to develop it, or what their expertise is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @RobTheOceanographer

Conflict of interest

Code of Conduct

General checks

  • Repository: Is the source for this learning module available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of a standard license? (OSI-approved for code, Creative Commons for content)
  • Version: Does the release version given match the repository release (v1.0.0)?
  • Authorship: Has the submitting author (@DamienIrving) made visible contributions to the module? Does the full list of authors seem appropriate and complete?

Documentation

  • A statement of need: Do the authors clearly state the need for this module and who the target audience is?
  • Installation instructions: Is there a clearly stated list of dependencies?
  • Usage: Does the documentation explain how someone would adopt the module, and include examples of how to use it?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the module 2) Report issues or problems with the module 3) Seek support

Pedagogy / Instructional design (Work-in-progress: reviewers, please comment!)

  • Learning objectives: Does the module make the learning objectives plainly clear? (We don't require explicitly written learning objectives; only that they be evident from content and design.)
  • Content scope and length: Is the content substantial for learning a given topic? Is the length of the module appropriate?
  • Pedagogy: Does the module seem easy to follow? Does it observe guidance on cognitive load? (working memory limits of 7 +/- 2 chunks of information)
  • Content quality: Is the writing of good quality, concise, engaging? Are the code components well crafted? Does the module seem complete?
  • Instructional design: Is the instructional design deliberate and apparent? For example, exploit worked-example effects; effective multi-media use; low extraneous cognitive load.

JOSE paper

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Does the paper clearly state the need for this module and who the target audience is?
  • Description: Does the paper describe the learning materials and sequence?
  • Does it describe how it has been used in the classroom or other settings, and how someone might adopt it?
  • Could someone else teach with this module, given the right expertise?
  • Does the paper tell the "story" of how the authors came to develop it, or what their expertise is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
@whedon
Copy link
Author

whedon commented Nov 21, 2018

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

⭐ Important ⭐

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/jose-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

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

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

@whedon
Copy link
Author

whedon commented Nov 21, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Nov 21, 2018

@labarba
Copy link
Member

labarba commented Nov 21, 2018

@darothen, @RobTheOceanographer 👋
Here's where the action happens. Let me know if you have any questions!

@darothen
Copy link

@labarba Thanks! I'll take a first pass through things over the holiday weekend and let you know if I have any questions. My goal is to be expeditious here, but could you provide an idea of deadlines you'd like us to hit for wrapping this up?

@labarba
Copy link
Member

labarba commented Nov 22, 2018

The ideal situation is a review lasting two or three weeks.

@darothen
Copy link

darothen commented Nov 25, 2018

Review / general comments

The materials presented here are a valuable contribution to the atmospheric/oceanic/climate science communities. They succinctly and simply cover a complete coding exercise which most students in the field will encounter routinely, and emphasizes best practices both with regards to scientific Python (e.g. using the "right tools" for the job), coding (using functions, building simple command line programs), software engineering (how to structure a re-usable piece of code, defensive coding), and development (using version control, re-factoring once you have an MWE). Furthermore, the materials are laid out in a logical way that most experienced coders in the field should easily be able to use to teach the material to their peers.

Pending a few small changes to the manuscript and supporting material which will address some of the JOSE requirements and improve their overall quality, I strongly encourage publication in JOSE.

Review pending checklist / specific comments

General Checks

  • Version - carpentrieslab/python-aos-lesson/issues/14
  • Authorship - carpentrieslab/python-aos-lesson/issues/15

Documentation

  • As a general comment here, the compiled module has good documentation for all of these topics, it's just not explicitly set out like in the JOSE paper. I think that's fine; at no point while reviewing the module did I feel lost or not understand why I was doing what I was doing.
  • I unfortunately did not have access to a Windows machine to easily review the Windows setup instructions.
  • For Usage, since this is a set of self-contained educational materials and documentation, I evaluated based on the ease of which it seemed to engage in a "self-guided" manner. I don't think there are any issues here, but it couldn't hurt to add a sentence or two to the index page of the module which indicates that a user can run through the material at their own pace, outside of a Data Carpentry event.

Pedagogy / Instructional design

Overall, this is a really well put together set of instructional materials. To directly respond to the author's request for comments, I've assembled the following short list of issues which will hopefully provide some recommendations for small improvements which will boost the overall utility and clarity of the instructional materials.

  • carpentrieslab/python-aos-lesson/issues/16
  • carpentrieslab/python-aos-lesson/issues/17

In the spirit of the JOSE paper, these issues may make good opportunities for collaborators to get involved with the core project and help flesh out and improve it. Triaging these issues as targets for new contributors would be an excellent way to address them.

JOSE Paper

  • carpentrieslab/python-aos-lesson/issues/18

@RobTheOceanographer
Copy link

RobTheOceanographer commented Dec 6, 2018

Review / general comments

Daniel's review picked up much of what I saw - great work @darothen! - nevertheless, I have some general comments and suggestions to contribute.

The atmospheric and oceanographic science communities are crying out for this type of training material. I am so glad that there are brilliant people like @DamienIrving who are going the extra distance to help bring them up to standard and to educate their peers and colleagues. The materials Damien present here are succinct and well thought out - it is a complete end-to-end experience for the students.

Pending the changes suggested by Daniel and a couple of suggestions I have below, I strongly recommend this manuscript for publication in JOSE.

Review pending checklist

The version doesn't match yet, but @darothen submitted an issue for it already: carpentrieslab/python-aos-lesson/issues/14

Lessons

In 02-visualisation the xr.open_dataset call threw an TypeError for me. This is likely due to an assumption that the conda env built by the users has the python netcdf4 package in it... but that was not the case for my set up, so I have submitted an issue to add netcdf4 to the install process in lesson 01-conda.

  • carpentrieslab/python-aos-lesson/issues/20

There are general inconsistencies in how you refer to relative paths - sometimes data/... other times .../data/.... While most users can out what you mean it would be better to go through the lessons and tidy this up:

  • carpentrieslab/python-aos-lesson/issues/21

In lesson 04-cmdline you refer to a code directory that does not exist if you follow the install/setup instructions provided. best to either ask users to put the .py scripts into a code dir or to refactor the paths to not refer to code/.... e.g. in lesson 04-cmdline the line $ cat code/script_template.py should become $ cat script_template.py. :

  • carpentrieslab/python-aos-lesson/issues/22

In lesson 07-vectorisation the first call to xarray.open_dataset has a syntax error as the xarray module is imported as xr:

  • carpentrieslab/python-aos-lesson/issues/23

I LOVE LESSON 08! This lesson must become mandatory for all science postgraduate students.

Pedagogy

The structure and pedagogy here are largely modelled on the software carpentry and data carpentry style. That is good because it follows a tried and true formula that has been proven to engage students and produce outcomes over many years.

The JOSE reviews guidelines say something about "The authors should briefly explain their 'Instructional design of the module' in the JOSE paper." I'm not sure that this is needed in this case as it is deeply linked to the carpentrieslab stuff.

@DamienIrving
Copy link

Thanks, @darothen and @RobTheOceanographer. It's been really useful to have you both take a detailed look over the lessons. I've gone through and addressed each of the issues you identified.

The following issues were simple bug fixes (each issue links to the relevant commit that addresses the problem):

One issue resulted in a substantial rewrite of the defensive programming lesson, which I think is now much improved:

One issue is a simple thing that I'll do next time I teach the materials (I'll leave the repo created during the GitHub lesson live on the web):

I'll fix the issue with the versioning as soon as the lessons are accepted for publication with JOSE (i.e. that will be release 1.0.0):

One issue resulted in a rewrite of the second paragraph of the JOSE paper, which is currently in the form of a pull request that I can merge if @darothen is satisfied with the changes:

@darothen
Copy link

darothen commented Dec 8, 2018

@whedon @labarba - not 100% certain what the follow-up process for reviews is, but I think @DamienIrving has done an excellent job responding to all of my comments and we're just about ready to publish.

@labarba
Copy link
Member

labarba commented Jan 1, 2019

@darothen, @RobTheOceanographer 👋 Happy New Year!
If you are satisfied with all the revisions, all we need from you is a statement here that you recommend publication. If you still find something is lacking, can you let us know? Thanks!

@darothen
Copy link

darothen commented Jan 1, 2019

@labarba - oh, sorry about that! I strongly recommend publication in the manuscript/materials' current form.

@RobTheOceanographer
Copy link

@labarba - yep. I recommend accepting the changes and publication. Yay team!

@labarba
Copy link
Member

labarba commented Jan 2, 2019

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jan 2, 2019

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jan 2, 2019

@DamienIrving
Copy link

The proofs look good to me. There was just one typo that I fixed up: carpentrieslab/python-aos-lesson@8828c78

@DamienIrving
Copy link

@labarba Now that I've checked the proofs (I fixed a minor typo as a result), is there anything else I need to do?

@labarba
Copy link
Member

labarba commented Jan 19, 2019

Thanks for the ping, @DamienIrving !

Please make an archive in Zenodo and post the DOI here. Make sure to edit the metadata (title and author list) to match your JOSS paper. Thanks!

@labarba
Copy link
Member

labarba commented Jan 19, 2019

Minor edits:

¶ 1 — comma after e.g.
¶ 2 — "it is also possible work through" > missing preposition "to"
comma after i.e.
(same at start of page 2, and ¶ 2 of page 2, and last paragraph)
period after etc

@DamienIrving
Copy link

@labarba Done 😄

DOI

@labarba
Copy link
Member

labarba commented Jan 21, 2019

@whedon set 10.5281/zenodo.2546005 as archive

@whedon
Copy link
Author

whedon commented Jan 21, 2019

OK. 10.5281/zenodo.2546005 is the archive.

@labarba
Copy link
Member

labarba commented Jan 21, 2019

@whedon accept

@whedon
Copy link
Author

whedon commented Jan 21, 2019

Attempting dry run of processing paper acceptance...

@whedon
Copy link
Author

whedon commented Jan 21, 2019

Check final proof 👉 openjournals/jose-papers#27

If the paper PDF and Crossref deposit XML look good in openjournals/jose-papers#27, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

@labarba
Copy link
Member

labarba commented Jan 21, 2019

@whedon accept deposit=true

@whedon
Copy link
Author

whedon commented Jan 21, 2019

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

@whedon
Copy link
Author

whedon commented Jan 21, 2019

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSE! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.jose.00037 jose-papers#28
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/jose.00037
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? notify your editorial technical team...

@labarba
Copy link
Member

labarba commented Jan 21, 2019

Yay! Congratulations, @DamienIrving, your paper is published in JOSE and the DOI is active.

A big thank you to your editor, @willingc, and your reviewers: @darothen, @RobTheOceanographer 🙏

@DamienIrving
Copy link

Thanks @willingc, @darothen, @RobTheOceanographer for the review and also @labarba for your efforts in establishing JOSE.

@labarba
Copy link
Member

labarba commented Jan 21, 2019

@labarba labarba closed this as completed Apr 22, 2019
@whedon
Copy link
Author

whedon commented Apr 22, 2019

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

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

Markdown:
[![DOI](https://jose.theoj.org/papers/10.21105/jose.00037/status.svg)](https://doi.org/10.21105/jose.00037)

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

reStructuredText:
.. image:: https://jose.theoj.org/papers/10.21105/jose.00037/status.svg
   :target: https://doi.org/10.21105/jose.00037

This is how it will look in your documentation:

DOI

We need your help!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants