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

Docs/add howto #144

Merged
merged 11 commits into from
Aug 10, 2022
Merged

Docs/add howto #144

merged 11 commits into from
Aug 10, 2022

Conversation

phackstock
Copy link
Contributor

Closes #142.
Opened in favor of #143.

This is my first attempt at a "how to" detailing the common pitfall of adding a model mapping without the corresponding region definition files.
As the "how to" referenced the nomenclature validate-project cli command, I added some additional docstring explanation that does not seem to be rendered currently by sphinx-click. Additionally, the direct referencing of a cli command seems to not be possible at the moment (https://sphinx-click.readthedocs.io/en/latest/usage/#cross-referencing) so I liked the CLI page.

A preview of the updated docs is rendered here: https://nomenclature-iamc.readthedocs.io/en/docs-add-howto/

@phackstock phackstock self-assigned this Aug 2, 2022
@phackstock phackstock added the documentation Improvements or additions to documentation label Aug 2, 2022
Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the first draft @phackstock. I made some suggestions, please also revise for more concise language.

Most importantly, I would frame this page as "Model registration", because we may want to add other types of model registration (e.g., use MAGICC for post-processing) here.

doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
@phackstock
Copy link
Contributor Author

Thanks for the first round of comments @danielhuppmann. I implemented them and made the language more concise. Hope that's a step in the right direction.
@luciecastella, please feel free to have a read at https://nomenclature-iamc.readthedocs.io/en/docs-add-howto/user_guide/howto.html and let me know if the explanation makes sense to you.

@luciecastella
Copy link
Contributor

The explanation makes sense yes! I find the difference between native and common regions hard to grasp in general and in this article, but that might just be because I do not work with the data. I'll let you see if it is useful to add something on that or if people working with the package are expected to know it.

@phackstock
Copy link
Contributor Author

phackstock commented Aug 10, 2022

@luciecastella thanks for the quick review. Good to hear that it made sense to you.
The difference between native regions and common regions is that the former are reported directly by the model itself and the latter are calculated, by us in this case, by summing up any number of model native regions. Common regions exist so that different models with different native spatial resolution can be compared.
Let's say we have two models a and b and we want to compare them. model_a natively reports a region called "Europe" while model_b reports "Western Europe" and "Eastern Europe". In this case we might want to define "Europe" as a common region and for model_b create it from summing up Western and Eastern Europe.
Hope that made sense.
Usually the modelers are familiar with the concept of native and common regions so I think in a quick how-to guide it should be fine to not explain it. In the more in-depth model mapping part of the documentation it is explained anyway. But maybe adding another reference might be a good idea.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two more minor suggestions inline, then good to be merged.

doc/source/user_guide.rst Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
doc/source/user_guide/howto.rst Outdated Show resolved Hide resolved
Co-authored-by: Daniel Huppmann <dh@dergelbesalon.at>
@danielhuppmann
Copy link
Member

Thanks @luciecastella for your comments. Please also use the "Approve" feature so that your approval can easily be seen in the status overview of the PR.

Based on your remarks, I started a new issue #145, which you and @phackstock can tackle in a follow-up PR.

Copy link
Contributor

@luciecastella luciecastella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good to me :)

@phackstock
Copy link
Contributor Author

I renamed the file from howto.rst to model-registration.rst and tried to make the sentence about the native region file more easily understandable. If that looks good to you @danielhuppmann, I'd go ahead with the merge then.
Also once we have a new release I think it would be a good idea to link this model registration section on the README for the workflows of each active project. I'd also add it to the workflow template repo.

@danielhuppmann
Copy link
Member

I think it would be a good idea to link this model registration section on the README for the workflows of each active project. I'd also add it to the workflow template repo.

Yes, good idea.

@phackstock phackstock merged commit 4a57d2f into main Aug 10, 2022
@phackstock phackstock deleted the docs/add-howto branch August 10, 2022 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add How-To section to the docs
3 participants