-
Notifications
You must be signed in to change notification settings - Fork 36
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
GemGIS - Spatial Data Processing for Geomodeling #128
Comments
Hello, |
Thank you @AlexanderJuestel for opening this and for linking back to #126. I have gone ahead and started looking for an editor for this review. Please do let us know when you get the API docs up, and then I will proceed with the initial checks we do before starting review. |
@NickleDave, I have created a question on Stackoverflow (https://stackoverflow.com/questions/76883029/api-reference-building-locally-but-not-on-rtd) and I am trying one last thing and hope to make it work! |
Not sure if I spotted the issue but I replied there: https://stackoverflow.com/a/76908429/4906855 You can also feel free to ask for help in our Discourse in the future (https://pyopensci.discourse.group/) (or now, if I haven't helped 😅) |
@NickleDave I am trying your solution right now! |
@NickleDave You solution did not work, unfortunately. I am only using two methods now to get a mini API Reference running but it also fails because a module, that is not even needed is missing. The only place this module appears is the I also opened a new topic on your Discourse! I hope someone has an idea! |
Hi @AlexanderJuestel just checking back. I think we do have an editor that can run this review. I know we were helping you with the API docs build but I'm a little out of the loop on where we left off. Could you give me a rough estimate of when you might have a chance to get to that? I don't mean to rush you--I'm sure you have other obligations as well; I just don't want to lose track of this review. My guess is that, if possible, switching to a pyproject.toml that includes your dependencies, and then using pure Python virtual environments on readthedocs, might make the build easier. But it's possible I'm missing the real issue. |
@NickleDave, thanks for checking in. I have to get back to @lwasser on Discourse still. It has been a little hectic here. I try to get back to her in the next few days! |
Very understood, thank you @AlexanderJuestel. |
@NickleDave Just to pick up the thread here again. The API Reference is working and the project was transferred to use a pyproject.toml file. I may also have some capacity to work on some other issues prior to the start of the review if necessary to ensure that the review itself is smoothly :) |
hey there @AlexanderJuestel just checking in on this review. we will keep this on hold until you are ready. so please just ping us. here (and also on slack) when you are ready to pick back up on the review. i think we might have an editor for this but it will depend on timing! |
@lwasser, sorry for being so unresponsive lately, I am getting into the final phase of my PhD. I have not made any progress on the tests yet. Would this be a reason to put the review on hold or can it be continued for now and I am completing the tests later? Cheers |
Hi @AlexanderJuestel, could you clarify a little?
I see you have a set of tests but it looks like those tests might not be passing. Is that what you mean? I can't find where we discussed tests previously.
If you are about to very busy with finishing up your PhD--believe me, I sympathize--then it might be better to put the review on hold. We don't want you to feel overwhelmed! We also need to make an effort to adhere to the timelines that we specify in our review process, so that we're not putting too much demands on our editors and reviewers. If you don't think you could respond to reviews in anywhere near the two week timeline, then now may not be a good time to start a review. I will admit we probably are taking longer than two weeks for many reviews, but still it's not fair to editors or reviewers to have to keep a review on their to-do list for months, so we should try to minimize that. Please say a little more about what's left to do with the tests, and whether you'll be able to meet the review timelines (roughly) given other things you have going on. edit: all that said, we do think we have an editor as @lwasser mentioned, so if you do feel like we could get through a review here, we do really want to support you with that after all the work you've done already! I know you've worked really hard to get GemGIS where it is now |
Hello @NickleDave, we mentioned the tests briefly on Slack (see below). I do not have 100% coverage with the tests right now. There are quite a few missing. If that would not be a show-stopper for now, can also proceed with the review as you suggested in your last paragraph. If not, we should put the review on hold for now and continue later. |
Thank you @AlexanderJuestel for clarifying, that helps. For tests, the criterion to start review is just that you have a test suite and it runs in CI. You have met that criterion. If I were a reviewer, I would definitely ask you to move towards full coverage as much as possible though. Let's go ahead and start the review. You've worked hard to get GemGIS ready, and I sense that you want to take this over the finish line. I'll reach out to the editor. |
@NickleDave sorry for getting back to so late. Yes, let us start the review! |
Hey @AlexanderJuestel ! |
Sorry I missed your reply @AlexanderJuestel! Thank you @yeelauren for taking over 🙏 |
Editor response to review:👋 @AlexanderJuestel hurrah! We've found reviewers. I like the look of the number of tutorials and examples included thus far, looking forward to more feedback from our reviewers :)Editor comments👋 Hi @aleksandraradecka1 and @martinfleis! Thank you for volunteering to review Please fill out our pre-review surveyBefore beginning your review, please fill out our pre-review survey. This helps us improve all aspects of our review and better understand our community. No personal data will be shared from this survey - it will only be used in an aggregated format by our Executive Director to improve our processes and programs.
The following resources will help you complete your review:
Please get in touch with any questions or concerns! Your review is due: ~March 29thReviewers: @aleksandraradecka1 @martinfleis |
@AlexanderJuestel @yeelauren I have edited the post above to include comments from the first round of the review. |
@martinfleis @aleksandraradecka1 @yeelauren Thank you for your reviews! If you don't mind, I would like to tackle these in two weeks after I have submitted my PhD thesis and can focus on programming a little bit :) |
Hey Everyone, I'll be ooo for this week and next. But wanted to introduce a second reviewer : @SimonMolinsky 👋🤝 |
Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Readme file requirements
The README should include, from top to bottom:
NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)
UsabilityReviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Functionality
For packages also submitting to JOSS
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
Final approval (post-review)
Estimated hours spent reviewing: 8 Review Comments |
@AlexanderJuestel Thank you for your work. It was a pleasure testing
When input arrays have different sizes, I get Import Error: The case when optional dependency should be listed as required.
"`python p.add_mesh(mesh=grid, scalars=grid["Elevation"], cmap='gist_earth') p.show_grid(color='black')
because my version of PyVista returned a key with the name The final view was inverted in my notebook:
Shouldn't there be a link from the sentence
"`python
Tutorial 4: Maybe cropping by a polygon of irregular shape? This has nothing to do with the package's functionality or the tutorial's quality.
|
@AlexanderJuestel 👋 Checking in on this since it's been a over a month since the last review! |
Yes, let's put this on hold for now. I am defending next Monday and will then slowly work on the suggestions and improvements :) |
The first point about renaming was addressed in cgre-aachen/gemgis@cbc9f31 (mentioned in cgre-aachen/gemgis#337) |
@ aleksandraradecka1, could you please elaborate on what you meant by this comment? :)
|
hi everyone 👋🏻 i'm just checking in on the status of this review! @AlexanderJuestel it looks like the comment above may not have pinged the reviewer as you might have intended to do. Please let me know if I can support this review moving forward. I hope that everyone here is well! |
Hey @AlexanderJuestel , I'm assuming the thesis defense is all wrapped up? (is that a loaded question? 😸 ) |
Submitting Author: Name (@AlexanderJuestel)
All current maintainers: (@AlexanderJuestel)
Package Name: GemGIS
One-Line Description of Package: Spatial Data Processing for Geomodeling
Repository Link: https://github.com/cgre-aachen/gemgis
Version submitted: 1.0.11 (soon 1.1 with bug fixes for some methods and the API Reference once it is working, no major functionality changes)
Documentation: https://gemgis.readthedocs.io/en/latest/
JOSS Publication: https://joss.theoj.org/papers/10.21105/joss.03709
JOSE Publication: https://jose.theoj.org/papers/10.21105/jose.00185
Editor: @yeelauren
Reviewer 1: @aleksandraradecka1
Reviewer 2: @martinfleis
Reviewer 3: @SimonMolinsky
Archive: TBD
Version accepted: TBD
Date accepted (month/day/year): TBD
Code of Conduct & Commitment to Maintain Package
Description
GemGIS is a Python-based, open-source geographic information processing library. It is capable of preprocessing spatial data such as vector data (shape files, geojson files, geopackages,…), raster data (tif, png,…), data obtained from online services (WCS, WMS, WFS) or XML/KML files (soon). Preprocessed data can be stored in a dedicated Data Class to be passed to the geomodeling package GemPy in order to accelerate the model building process. Postprocessing of model results will allow export from GemPy to geoinformation systems such as QGIS and ArcGIS or to Google Earth for further use.
GemGIS uses and combines the full functionality of GeoPandas, rasterio, OWSLib, Pandas, Shapely, PyVista and NumPy to simplify, accelerate and automate the workflows used to preprocess spatial data for geomodeling.
From https://gemgis.readthedocs.io/en/latest/
In addition, almost 70 tutorials illustrate the different functionalities of GemGIS.
Scope
Please indicate which category or categories.
Check out our package scope page to learn more about our
scope. (If you are unsure of which category you fit, we suggest you make a pre-submission inquiry):
Domain Specific & Community Partnerships
Community Partnerships
If your package is associated with an
existing community please check below:
Data extraction/processing: GemGIS uses the functionality of packages like GeoPandas, Shapely, or Rasterio to extract information from vector and raster data and to put it in a form that it can be used by the GemPy.
Data visualization: GemGIS uses the functionality of packages like matplotlib or PyVista to create static and dynamic plots of data and meshes. This includes digital elevation models or meshes of subsurfaces layers, boreholes geological cross sections or even seismic data.
Workflow automation: The entire purpose of GemGIS is to provide methods to accelerate the preparation of input data for GemPy. Over time, the package has also gained additional functionality to work with a variety of datasets utilized for subsurface applications, see Tutorials.
The target audience is the open-source Geosciences community, researchers, students but also industry. GemGIS provides functionality to accelerate the preparation of input data for the structural geological modeling package GemPy which has been used in numerous publications. For applications at universities, we are in the final stages of getting a JOSE publication approved with more than 20 structural geological models that are used at RWTH Aachen University for teaching purposes and where GemGIS and GemPy will be included in future courses.
GemGIS does not reinvent the wheel but rather combines the functionality of already existing packages mentioned in the description above. The packages utilized the most in GemGIS are the well-known packages like GeoPandas, Shapely, Rasterio, Pandas, NumPy, PyVista, matplotlib, etc. We also decided against i.e. wrapping GeoPandas GeoDataFrames in our own class or creating many new classes so that users can still use the full functionality of the underlying packages. This is one big advantage in comparison to GemPy where i.e. the meshes of the resulting structural geological models cannot be extracted (GemGIS is capable of extracting them though). Another example is that raster data opened with GemGIS will be stored as PyVista PolyData datasets or as grids so that users can harvest the functionality of this amazing package.
@tag
the editor you contacted:Pre-submission inquiry: #126, @NickleDave
Technical checks
For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:
Publication Options
JOSS Checks
paper.md
matching JOSS's requirements with a high-level description in the package root or ininst/
.Note: JOSS accepts our review as theirs. You will NOT need to go through another full review. JOSS will only review your paper.md file. Be sure to link to this pyOpenSci issue when a JOSS issue is opened for your package. Also be sure to tell the JOSS editor that this is a pyOpenSci reviewed package once you reach this step.
Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?
This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.
Confirm each of the following by checking the box.
Please fill out our survey
submission and improve our peer review process. We will also ask our reviewers
and editors to fill this out.
P.S. Have feedback/comments about our review process? Leave a comment here
Editor and Review Templates
The editor template can be found here.
The review template can be found here.
Footnotes
Please fill out a pre-submission inquiry before submitting a data visualization package. ↩
The text was updated successfully, but these errors were encountered: