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

Documentation improvement for TaylorDiagram class and set_titles_and_labels #219

Merged
merged 16 commits into from
Mar 23, 2024

Conversation

jukent
Copy link
Collaborator

@jukent jukent commented Feb 22, 2024

PR Summary

Closes #176 and #212

PR Checklist

General

  • Make an issue if one doesn't already exist
  • Link the issue this PR resolves by adding closes #XXX to the PR description where XXX is the number of the issue.
  • Add a brief summary of changes to docs/release-notes.rst in a relevant section for the next unreleased release. Possible sections include: Documentation, New Features, Bug Fixes, Internal Changes, Breaking Changes/Deprecated
  • Add appropriate labels to this PR
  • Make your changes in a forked repository rather than directly in this repo
  • Open this PR as a draft if it is not ready for review
  • Convert this PR from a draft to a full PR before requesting reviewers
  • Passes precommit. To set up on your local, run pre-commit install from the top level of the repository. To manually run pre-commits, use pre-commit run --all-files and re-add any changed files before committing again and pushing.

Documentation

  • Docstrings have been added to all new functions
  • Docstrings have been updated with any function changes
  • Internal functions have a preceding underscore (_) and have been added to docs/internal_api/index.rst
  • User facing functions have been added to docs/user_api/index.rst under their module

Examples

  • Any new notebook examples added to docs/examples/ folder
  • Clear all notebook cells
  • New notebook files added to docs/examples.rst toctree
  • New notebook files added to new entry in docs/gallery.yml with appropriate thumbnail photo in docs/_static/thumbnails/

@jukent jukent changed the title add note on taylor diagram size limitations Documentation improvement for TaylorDiagram class and set_titles_and_labels Mar 18, 2024
@jukent jukent added the documentation Improvements or additions to documentation label Mar 18, 2024
@jukent jukent marked this pull request as ready for review March 18, 2024 13:49
Copy link
Contributor

@kafitzgerald kafitzgerald left a comment

Choose a reason for hiding this comment

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

Agree this makes more sense.

A few typos and suggestions mentioned in the comments, but mostly wanted to raise a few questions about terminology (while we're making painful changes).

I wonder if we want to add some sort of deprecation warnings as well. Thoughts?

docs/release-notes.rst Outdated Show resolved Hide resolved
docs/release-notes.rst Outdated Show resolved Hide resolved
src/geocat/viz/taylor.py Outdated Show resolved Hide resolved
src/geocat/viz/taylor.py Outdated Show resolved Hide resolved
src/geocat/viz/taylor.py Outdated Show resolved Hide resolved
src/geocat/viz/util.py Outdated Show resolved Hide resolved
jukent and others added 5 commits March 20, 2024 14:24
Copy link
Contributor

@kafitzgerald kafitzgerald left a comment

Choose a reason for hiding this comment

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

I might have spaced this before (apologies if so), but we should probably add either deprecation warnings or pending deprecation warnings and something in a deprecations section of the release notes about the renamed methods.

Otherwise, looks good.

@jukent
Copy link
Collaborator Author

jukent commented Mar 22, 2024

I might have spaced this before (apologies if so), but we should probably add either deprecation warnings or pending deprecation warnings and something in a deprecations section of the release notes about the renamed methods.

Otherwise, looks good.

Thanks @kafitzgerald just added them!

"""

warnings.warn(
'This function is deprecated. Call `add_std_grid` instead.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'This function is deprecated. Call `add_std_grid` instead.',
'`TaylorDiagram.add_ygrid` will be deprecated in the future. Please use `TaylorDiagram.add_std_grid` instead.',

"""

warnings.warn(
'This function is deprecated. Call `add_corr_grid` instead.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'This function is deprecated. Call `add_corr_grid` instead.',
'`TaylorDiagram.add_xgrid` will be deprecated in the future. Please use `TaylorDiagram.add_corr_grid` instead.',

by array *arr*
"""Add gridlines to the correlation axis specified by array *arr*.

This function will be deprecated in favor of `add_corr_grid()`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This function will be deprecated in favor of `add_corr_grid()`
This method will be deprecated in favor of `TaylorDiagram.add_corr_grid`


*arr*.

This function will be deprecated in favor of `add_std_grid()`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This function will be deprecated in favor of `add_std_grid()`
This method will be deprecated in favor of `TaylorDiagram.add_std_grid`.

docs/release-notes.rst Outdated Show resolved Hide resolved
@jukent jukent merged commit 4f261e0 into NCAR:main Mar 23, 2024
15 checks passed
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.

Taylor Diagram class is not very robust
2 participants