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

Cleanup recipe headers before the release #1740

Merged
merged 8 commits into from Jul 22, 2020
Merged

Conversation

mattiarighi
Copy link
Contributor

@mattiarighi mattiarighi commented Jul 20, 2020

Make recipe headers uniform and fix a few typos.

@mattiarighi mattiarighi added this to the v2.0.0 milestone Jul 20, 2020
@mattiarighi mattiarighi requested a review from bouweandela Jul 20, 2020
@mattiarighi
Copy link
Contributor Author

mattiarighi commented Jul 22, 2020

@katjaweigel I renamed recipe_martin2018grl.yml to recipe_martin18grl.yml for consistency with our file naming convention.

@mattiarighi mattiarighi requested a review from katjaweigel Jul 22, 2020
- ewatercycle

references:
- sutanudjaja2018gmd
Copy link
Member

@bouweandela bouweandela Jul 22, 2020

Choose a reason for hiding this comment

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

You lost acknow_project here

Copy link
Member

@bouweandela bouweandela Jul 22, 2020

Choose a reason for hiding this comment

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

Though that is actually not a reference, but a request to acknowledge, so maybe we need to think a bit about how to improve this in the future

Copy link
Contributor Author

@mattiarighi mattiarighi Jul 22, 2020

Choose a reason for hiding this comment

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

acknow_project was introduced for cases where no other references were available, which is not case here.

Copy link
Member

@bouweandela bouweandela Jul 22, 2020

Choose a reason for hiding this comment

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

That's probably never the case, because all our recipes are based on scientific work, right? Only the example recipes wouldn't have references.

Copy link
Contributor Author

@mattiarighi mattiarighi Jul 22, 2020

Choose a reason for hiding this comment

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

Actually there are 5 recipes without an actual reference: recipe_consecdrydays.yml, recipe_ocean_Landschuetzer2016.yml, recipe_ocean_multimap.yml, recipe_shapeselect.yml and recipe_spei.yml.

Copy link
Member

@bouweandela bouweandela Jul 22, 2020

Choose a reason for hiding this comment

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

Maybe references just need to be added?

Copy link
Contributor Author

@mattiarighi mattiarighi Jul 22, 2020

Choose a reason for hiding this comment

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

Maybe yes, but not all our diagnostics are based on published work, or the paper is not yet ready.

Copy link
Member

@bouweandela bouweandela Jul 22, 2020

Choose a reason for hiding this comment

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

I think it would be fine to have no reference if no reference is available.

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
@katjaweigel
Copy link
Contributor

katjaweigel commented Jul 22, 2020

For recipe_spei.yml there are the references vicente10jclim and mckee93proc given inside the two diagnostics it calls. But they are references for the SPEI and SPI indices used, not for the histograms of them plotted by the recipe, this is probably why they are not there at the moment.

@mattiarighi mattiarighi merged commit 74fe96f into master Jul 22, 2020
1 check passed
@mattiarighi mattiarighi deleted the recipe_cleanup branch Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants