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]: From Maps to Models - Tutorials for structural geological modeling using GemPy and GemGIS #185

Closed
44 tasks done
whedon opened this issue Dec 1, 2022 · 117 comments
Closed
44 tasks done
Assignees
Labels
accepted Jupyter Notebook published Papers published in JOSE Python QML recommend-accept Papers recommended for acceptance in JOSE. review TeX

Comments

@whedon
Copy link

whedon commented Dec 1, 2022

Submitting author: @AlexanderJuestel (Alexander Jüstel)
Repository: https://github.com/cgre-aachen/gemgis_data
Branch with paper.md (empty if default branch):
Version: 1.0.0
Editor: @acocac
Reviewers: @BenjMy, @jwagemann
Archive:

Status

status

Status badge code:

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

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@BenjMy & @jwagemann, 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://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @acocac know.

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

Review checklist for @BenjMy

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: 1.0.0
  • Authorship: Has the submitting author (@AlexanderJuestel) 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 @jwagemann

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: 1.0.0
  • Authorship: Has the submitting author (@AlexanderJuestel) 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 Dec 1, 2022

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @BenjMy, @jwagemann it looks like you're currently assigned to review this paper 🎉.

⚠️ JOSE reduced service mode ⚠️

⭐ 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

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

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Dec 1, 2022

Wordcount for paper.md is 2280

@whedon
Copy link
Author

whedon commented Dec 1, 2022

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=2.14 s (31.8 files/s, 52267.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Jupyter Notebook                49              0         105485           5289
Markdown                         7            107              0            268
QML                              1              3              0            240
XML                              8              0              0            221
TeX                              1             14              0            168
YAML                             2              5              6             34
-------------------------------------------------------------------------------
SUM:                            68            129         105491           6220
-------------------------------------------------------------------------------


Statistical information for the repository '537c091cba9272b7a32d1dce' was
gathered on 2022/12/01.
No commited files with the specified extensions were found.

@whedon
Copy link
Author

whedon commented Dec 1, 2022

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@whedon
Copy link
Author

whedon commented Dec 1, 2022

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5281/zenodo.4569086 is OK
- 10.5194/gmd-12-1-2019 is OK
- 10.21105/joss.01450 is OK
- 10.5281/zenodo.4572994 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.1016/j.pepi.2008.06.013 is OK
- 10.1007/978-94-011-2556-7_11 is OK
- 10.1130/GES02455.1 is OK
- 10.1007/BF02775087 is OK
- 10.1007/978-1-4615-9630-1 is OK
- 10.21105/joss.03709 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@acocac
Copy link

acocac commented Dec 1, 2022

👋 @BenjMy & @jwagemann we will conduct the review in this issue.

Please read through the above information and let me know if you have any questions about process.

Thank you 🙏

@whedon
Copy link
Author

whedon commented Dec 15, 2022

👋 @BenjMy, please update us on how your review is going (this is an automated reminder).

@whedon
Copy link
Author

whedon commented Dec 15, 2022

👋 @jwagemann, please update us on how your review is going (this is an automated reminder).

@BenjMy
Copy link

BenjMy commented Dec 16, 2022

Thanks for the reminder, I should be able to send my review by the end of next week.

@jwagemann
Copy link

Same here - should be able to submit it next week.

@BenjMy
Copy link

BenjMy commented Dec 19, 2022

@AlexanderJuestel @acocac

I have completed an initial review of the GemGIS tutorial. First I'd like to congratulate the GemGIS dev team for this nice piece of work. I took the measure of all the time and investment made to build the tools and notebooks and make them accessible for everyone.

This package has the potential to make an immediate impact on the scientific communities. There are thoughtful and well-paced tutorial notebooks that provide high-level introductions and more detailed examples of the various methods offered in GemGIS (and Gempy). I am excited both for the package's potential in the long run and possibly to use it in the immediate future.

Below are some of my initial thoughts from the checklist that I will begin creating issues for and linking back to this comment.

Checklist topics

  • General Checks

    • Version: up to now there is no release of the proposed package.
    • Contribution and authorship: unclear what role Miguel de la Varga, Nils Chudalla, and Stefan Back have played in the code development (no commits).
  • Documentation

    • Readers are invited to refer to the GEMGIS core code documentation since there is no specific documentation for the GEMGIS-data repos. I think hosting the datasets and notebooks in a separate repo is perfectly relevant particularly to provide clear guidelines on the data FAIRness.

      • I suggest adding details (add a section in the documentation) covering how FAIR are the dataset provided:
      • The LICENCE file covers the GemGIS software rights. But under what Licence are the data distributed? I've seen that the authors cited individually the map source for each notebook but it should also clearly be stated on the README and in the doc.
      • Is there a plan for long-term archiving of the data? into Zenodo for instance?
      • Is there a plan for version-controlled data? data could be fetched (using Pooch for example).
    • A statement of need

      • The authors don't identify the target audience in the docs nor in the README.
      • The definition of the problem being solved is only part of the doc. Would be also interesting to add it to the README.
    • Installation instructions

      • Issues with the badge target link
      • The is no list of dependencies in the README. At least the authors should make clear that Gempy is an optional dependency.
      • Error using conda-forge installation (inconsistent env)
      • Error using pip installation (missing dependencies)
      • In the doc, reorder and put "Installing Gempy (Optional)" at the end of the list.
    • Usage

      • I opened an issue with some recommendations.
      • Error Using Binder when GEMGIS is imported.
    • Community guidelines:

      • Add "Authorship and credit" statement.
      • I would detail a bit more what can of data can be submitted. Under what terms?
  • Pedagogy / Instructional design

    • Learning objectives

    Individually each notebook is thoroughly documented to guide the user and it is really much appreciated. Nevertheless, the documentation design with respect to the learning objective can be improved. I recommend the authors articulate the notebooks over sections and subsections. Ideally, I would distinguish within new section tutorials which are Gempy dependent from the others. Also, you could articulate the notebooks using sections following your "Learning Objectives" workflow.

    • Instructional design
      • Video Tutorials are expected and I think it is a great initiative.
  • Jose Paper

Overall it is a good start but it reads a bit choppy. I found the article very resource exploitation (oil, mining, ...) target-oriented while I think this tool has many other disciplines to reach.

  • Statement of needs

    • To me for instance the tools developed can be also associated with environmental risk management (water scarcity, landslides, ...). I am personally interested in this package for Catchment hydrogeology modelling. Authors should broaden this idea in the statement of needs.

    • I like the proof of concept and that the authors already used the software to teach RWTH Aachen. Could it be possible to add (in the "Experience of use in teaching and learning situations" section):

      • a link to the degree where it has been taught;
      • the number of hours dedicated to the course (and how it has been distributed over all the notebooks);
      • the ratio between the number of instructors and the number of students.
    • "Even from seismic data" is to me again to me narrow. Why other geophysical approaches able to infer geological structures (TEM, gravity, ...) are not considered?

  • Ressources:

  • References: it looks like the references were broken during the paper compilation.

@jwagemann
Copy link

@acocac I also concluded my review and added my comments to the issues @BenjMy opened in the target repository.

I'd like to thank the authors for submitting their tutorial to JOSE and I think that the notebooks have a great potential to be a valuable contribution for the GemGIS and GemPy communities. However, at this stage, my evaluation is that the tutorials require a major review as in my opinion pedagogical and educational standards of notebooks are not met.

There is room to improve the structure of the course, but also the quality of the notebooks in terms of including a pedagogical narrative. Once the notebooks have been better structured and improved, the paper submission needs a revision as well. There is also potential to improve structure, clarity and guidance.

My proposals on how to improve the submission are found as a response to the following issues:

@AlexanderJuestel
Copy link

Dear @BenjMy,
Dear @jwagemann,

thank you very much for your extensive reviews. I will try to resolve all issues during the Christmas break! We certainly have a lot of things to do here!

Cheers
Alex

@acocac
Copy link

acocac commented Dec 20, 2022

Hello, @AlexanderJuestel 👋
@jwagemann @BenjMy have kindly provided their feedback and suggestions to improve the submission.
Please work on your revisions and come back here to let us know when you want reviewers and myself to take a new look.
Wishing a happy Christmas break.

@labarba
Copy link
Member

labarba commented Aug 8, 2023

Hi everybody! 👋
Thank you all for your patience. I looked over the paper, and see that the citations need fixing.
@AlexanderJuestel – Please have a look a the Markdown citation format we use.

@labarba
Copy link
Member

labarba commented Aug 8, 2023

From our docs:

Citations

Citations to entries in paper.bib should be in
rMarkdown
format.

For a quick reference, the following citation commands can be used:

  • @author:2001 -> "Author et al. (2001)"
  • [@author:2001] -> "(Author et al., 2001)"
  • [@author1:2001; @author2:2001] -> "(Author1 et al., 2001; Author2 et al., 2002)"

@AlexanderJuestel
Copy link

@whedon generate pdf

@editorialbot
Copy link
Collaborator

My name is now @editorialbot

@AlexanderJuestel
Copy link

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@AlexanderJuestel
Copy link

AlexanderJuestel commented Aug 8, 2023

@labarba I have fixed the citation commands in the manuscript and also adapted the rules for capitalization in titles in the bibtex file.

Let me know if there is anything else.

Cheers Alex

@labarba
Copy link
Member

labarba commented Aug 8, 2023

Ah, yes. Some links are broken too. Have a look at the bottom of page 7.

@AlexanderJuestel
Copy link

Thanks for pointing that out, @labarba. The links should be fixed now!

@labarba
Copy link
Member

labarba commented Aug 8, 2023

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator

@labarba
Copy link
Member

labarba commented Aug 8, 2023

We have another error. Please check the report above. It says:

Error reading bibliography file paper.bib: (line 175, column 1): unexpected '@'

@AlexanderJuestel
Copy link

Yeah, forgot to merge my PR, sorry

@AlexanderJuestel
Copy link

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@labarba
Copy link
Member

labarba commented Aug 9, 2023

This is such an impressive piece of work!

I could publish as is, but maybe you want to correct a couple of remaining details: on p.3, line 119, the phrase "for the basic structural geological," seems to be missing a noun that follows, like "model" or "map" or something like that - on p.6, line 165, "the open-source developed 3D creation suite Blender" sounds awkward; maybe just delete "developed" - in the Bibliography, use brackets to protect the author display that now awkwardly shows "Community, B. O"

@AlexanderJuestel
Copy link

Thank you! The comments of both reviewers improved the quality of the entire submission.

I fixed your remaining comments!

@labarba
Copy link
Member

labarba commented Aug 10, 2023

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@labarba
Copy link
Member

labarba commented Aug 15, 2023

@editorialbot accept

@editorialbot
Copy link
Collaborator

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

@editorialbot
Copy link
Collaborator

Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository.

If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file.

You can copy the contents for your CITATION.cff file here:

CITATION.cff

cff-version: "1.2.0"
authors:
- family-names: Jüstel
  given-names: Alexander
  orcid: "https://orcid.org/0000-0003-0980-7479"
- family-names: Varga
  given-names: Miguel
  name-particle: de la
  orcid: "https://orcid.org/0000-0001-6941-2685"
- family-names: Chudalla
  given-names: Nils
- family-names: Wagner
  given-names: Jan David
  orcid: "https://orcid.org/0009-0007-7673-8334"
- family-names: Back
  given-names: Stefan
  orcid: "https://orcid.org/0000-0003-3134-3367"
- family-names: Wellmann
  given-names: Florian
  orcid: "https://orcid.org/0000-0003-2552-1876"
message: If you use this software, please cite our article in the
  Journal of Open Source Software.
preferred-citation:
  authors:
  - family-names: Jüstel
    given-names: Alexander
    orcid: "https://orcid.org/0000-0003-0980-7479"
  - family-names: Varga
    given-names: Miguel
    name-particle: de la
    orcid: "https://orcid.org/0000-0001-6941-2685"
  - family-names: Chudalla
    given-names: Nils
  - family-names: Wagner
    given-names: Jan David
    orcid: "https://orcid.org/0009-0007-7673-8334"
  - family-names: Back
    given-names: Stefan
    orcid: "https://orcid.org/0000-0003-3134-3367"
  - family-names: Wellmann
    given-names: Florian
    orcid: "https://orcid.org/0000-0003-2552-1876"
  date-published: 2023-08-15
  doi: 10.21105/jose.00185
  issn: 2577-3569
  issue: 66
  journal: Journal of Open Source Education
  publisher:
    name: Open Journals
  start: 185
  title: From Maps to Models - Tutorials for structural geological
    modeling using GemPy and GemGIS
  type: article
  url: "https://jose.theoj.org/papers/10.21105/jose.00185"
  volume: 6
title: From Maps to Models - Tutorials for structural geological
  modeling using GemPy and GemGIS

If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation.

Find more information on .cff files here and here.

@editorialbot
Copy link
Collaborator

🚨🚨🚨 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.00185 jose-papers#134
  2. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/jose.00185
  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...

@editorialbot editorialbot added accepted published Papers published in JOSE labels Aug 15, 2023
@labarba
Copy link
Member

labarba commented Aug 15, 2023

Congratulations @AlexanderJuestel, your JOSE paper is published! 🚀

Huge thanks to our Editor: @acocac and Reviewers: @BenjMy, @jwagemann — your contributions make this possible 🙏

@labarba labarba closed this as completed Aug 15, 2023
@editorialbot
Copy link
Collaborator

🎉🎉🎉 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.00185/status.svg)](https://doi.org/10.21105/jose.00185)

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

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

This is how it will look in your documentation:

DOI

We need your help!

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

@AlexanderJuestel
Copy link

@labarba great news! Thank you very much! Thank you to both reviewers (@jwagemann, @BenjMy) and to @acocac for managing everything!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Jupyter Notebook published Papers published in JOSE Python QML recommend-accept Papers recommended for acceptance in JOSE. review TeX
Projects
None yet
Development

No branches or pull requests

7 participants