8000 all of the 'chainging_func_kwargs' are changed to 'chaining_func_kwargs' by JinghanFu · Pull Request #772 · nci/scores · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

all of the 'chainging_func_kwargs' are changed to 'chaining_func_kwargs' #772

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

Conversation

JinghanFu
Copy link
Contributor

Please work through the following checklists. Delete anything that isn't relevant.

Development for new xarray-based metrics

  • Works with n-dimensional data and includes reduce_dims, preserve_dims, and weights args.
  • Typehints added
  • Add error handling
  • Imported into the API
  • Works with both xr.DataArrays and xr.Datasets if possible

Docstrings

  • Docstrings complete and follow Napoleon (google) style
  • Maths equation added
  • Reference to paper/webpage is in docstring. The preferred referencing style for journal articles is APA (7th edition)
  • Code example added

Testing of new xarray-based metrics

  • 100% unit test coverage
  • Test that metric is compatible with dask.
  • Test that metrics work with inputs that contain NaNs
  • Test that broadcasting with xarray works
  • Test both reduce and preserve dims arguments work
  • Test that errors are raised as expected
  • Test that it works with both xr.DataArrays and xr.Datasets

Tutorial notebook

  • Short introduction to why you would use that metric and what it tells you
  • A link to a reference
  • A "things to try next" section at the end
  • Add notebook to Tutorial_Gallery.ipynb
  • Optional - a detailed discussion of how the metric works at the end of the notebook

Documentation

@tennlee tennlee linked an issue Nov 26, 2024 that may be closed by this pull request
@JinghanFu JinghanFu force-pushed the 770-change-mispelled-argument-name-for-method-tw_crps_for_ensemble branch from 5f3a5d5 to 57ae3ed Compare November 26, 2024 03:40
@tennlee
Copy link
Collaborator
tennlee commented Nov 26, 2024

Thanks @JinghanFu, please add a note indicating the name you would like used for the release notes, and also consider adding yourself to the .zenodo.json.

@Steph-Chong
Copy link
Collaborator

@JinghanFu - I know @tennlee has already discussed this with you. But I am pasting some information below, just so it is readily available :-).

Thank you very much for your contribution.

When we release a new version of scores, that version is archived on Zenodo. See: https://doi.org/10.5281/zenodo.12697241

As you have contributed to scores, would you like to be listed on Zenodo as an author the next time scores is archived?

If so, please open a new pull request. In that pull request please add your details to .zenodo.json (which can be found in the scores root directory).

In .zenodo.json, please add your details at the bottom of the “creators” section. The fields you will need to complete are:

  1. “orcid”. This is an optional field. If you don’t have an ORCID, but would like one, you can obtain one here: https://info.orcid.org/researchers/ .
  2. “affiliation”. Options include: the institution you are affiliated with, “Independent Researcher” or “Independent Contributor”.
  3. “name”. Format: surname, given name(s).

@JinghanFu
Copy link
Contributor Author

Thank you so much, @tennlee @Steph-Chong!! I will open a new pull request to include my name on Zenodo and release notes!

@Steph-Chong Steph-Chong mentioned this pull request Nov 26, 2024
23 tasks
Signed-off-by: Tennessee Leeuwenburg <134973832+tennlee@users.noreply.github.com>
@tennlee
Copy link
Collaborator
tennlee commented Nov 26, 2024

Hi @JinghanFu , I had to do a small tidyup on this PR. To do that, I needed to pull in this change onto my fork, do the tidy up, and then merge it from there. See PR #780 for where that occurred. So we have included your work, and the commit shows up with you as a co-author. I've also added you to the .zenodo.json by merging your pull request #777. Thanks for this, very helpful! I will now close this PR (#772) as well.

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

Successfully merging this pull request may close these issues.

Mis-spelled argument name
3 participants
0