Feature #557 line, eclv, equivalance testing bounds, revision series, and contour - plotly to matplotlib#571
Open
georgemccabe wants to merge 117 commits intofeature_555_replace_plotlyfrom
Open
Feature #557 line, eclv, equivalance testing bounds, revision series, and contour - plotly to matplotlib#571georgemccabe wants to merge 117 commits intofeature_555_replace_plotlyfrom
georgemccabe wants to merge 117 commits intofeature_555_replace_plotlyfrom
Conversation
* Updated the release_date * Removed the -rc1 suffix for coordinated release * Updated for coordinated release * Rotate authorship for coordinated release * Update year of copyright date * Per #550, modify formatting of release notes header * Per #550, attempting to resolve SonarQube error with regard to coverage Add source and omit settings for coverage tool * Per #550, correcting syntax Updated source paths for coverage tool and adjusted omit settings. * Per #550, update coverage omit pattern for _netCDF4.pyx * Per #550, another attempt to resolve the SonarQube error * Per #550, another attempt to resolve the SonarQube error Updated coverage configuration to omit specific files. * Update pyproject.toml move the omit from the tool.coverage.report to the tool.coverage.run to see if the histogram_2d/src/netCDF4 can be omitted from the trace * Update pyproject.toml try [run] source=. to prevent measuring code outside of project * remove the [run] block, this causes issues with another GHA * add source=. to only check the METplotpy source code and not third party libraries * Use different syntax to omit the histogram_2d/src from report * Return to original state, moved omit to the .coveragec config * Update pyproject.toml remove extraneous line * Update release-notes.rst * Update conf.py --------- Co-authored-by: Julie Prestopnik <jpresto@ucar.edu>
… Matplotlib support
…g.py. Updated plots that use plotly to import from those versions so it is clear which plots still rely on plotly and need to be updated. This will also allow optional support of plotly for certain plots if we are not able to fully get rid of the plotly dependency in this development cycle. Also removed some unused imports. Replaced util.py function apply_weight_style with get_font_params since the existing version will not be able to be used with matplotlib
…y matplotlib. Instead set xaxis label weight similar to taylor_diagram logic
…and_common_functionality
…feature_556_copy_base_and_common_functionality
…tring 9999 to integer 9999 and use np.nan instead of string 'NA'
…causes yaml configurations for lines to be ignored
…feature_556_copy_base_and_common_functionality
…and_common_functionality
update copyright year
…s://github.com/dtcenter/METplotpy into feature_556_copy_base_and_common_functionality
…feature_556_copy_base_and_common_functionality
…ght date and information.
…and_common_functionality
…units. added helper function to reduce duplication for logic to convert units
…plotly version for now
…l and y-axis label style, weight, and font size. Moved the add_horizontal_line() and add_vertical_line() code from the util.py module to this module as this will be needed for all plot types. TODO comments are used to denote code that will need to be removed when all plot types have migratee to Matplotlib.
…s://github.com/dtcenter/METplotpy into feature_556_copy_base_and_common_functionality
…to properly handle the x value locations for some plots to use the actual values instead of an index 0-n
…h from the line plot as possible. Created function in line plot to handle drawing of series so that it could be used for the eclv line plots that draw multiple lines for pct series
…y match plotly default
georgemccabe
changed the title
23 tasks
bikegeek
reviewed
Mar 20, 2026
| if self.x_tickangle in constants.XAXIS_ORIENTATION.keys(): | ||
| self.x_tickangle = constants.XAXIS_ORIENTATION[self.x_tickangle] | ||
| # self.x_tickangle = self.parameters['xtlab_orient'] | ||
| # if self.x_tickangle in constants.XAXIS_ORIENTATION.keys(): |
Collaborator
There was a problem hiding this comment.
is line 65 supposed to be commented out, or is the tick label orientation for the x-axis not supported?
Contributor
Author
There was a problem hiding this comment.
Thanks for bringing this up. I left this commented because I wasn't sure how to handle this.
The XAXIS_ORIENTATION constant only maps values to 0 or 270. The plotly revision_series had the x-tick label orientation hard-coded to 90, so it was not possible to get the test config to map to the correct orientation. When I changed this plot to use the _add_xaxis I added to base_plot.py to handle these consistently, it could not be oriented properly.
XAXIS_ORIENTATION = {0: 0, 1: 0, 2: 270, 3: 270}
I wanted to check why 0-1 map to 0 and 2-3 map to 270 and if that is correct. Does METviewer set these orientation 0-3 integers?
Collaborator
|
Let me dig into the METviewer code and see whether this is set up somewhere
in the Java code and what the original R code did.
…On Fri, Mar 20, 2026 at 10:32 AM George McCabe ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In metplotpy/plots/revision_series/revision_series_config.py
<#571 (comment)>:
> @@ -62,11 +62,11 @@ def __init__(self, parameters: dict) -> None:
##############################################
# x-axis parameters
self.x_title_font_size = self.parameters['xlab_size'] + constants.DEFAULT_TITLE_FONTSIZE
- self.x_tickangle = self.parameters['xtlab_orient']
- if self.x_tickangle in constants.XAXIS_ORIENTATION.keys():
- self.x_tickangle = constants.XAXIS_ORIENTATION[self.x_tickangle]
+ # self.x_tickangle = self.parameters['xtlab_orient']
+ # if self.x_tickangle in constants.XAXIS_ORIENTATION.keys():
Thanks for bringing this up. I left this commented because I wasn't sure
how to handle this.
The XAXIS_ORIENTATION constant only maps values to 0 or 270. The plotly
revision_series had the x-tick label orientation hard-coded to 90, so it
was not possible to get the test config to map to the correct orientation.
When I changed this plot to use the _add_xaxis I added to base_plot.py to
handle these consistently, it could not be oriented properly.
XAXIS_ORIENTATION = {0: 0, 1: 0, 2: 270, 3: 270}
I wanted to check why 0-1 map to 0 and 2-3 map to 270 and if that is
correct. Does METviewer set these orientation 0-3 integers?
—
Reply to this email directly, view it on GitHub
<#571?email_source=notifications&email_token=AA4UJHXNZNBCEYYZPNNFXFT4RVXADA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTGOJYGI3DKMRRG432M4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#discussion_r2966758049>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4UJHW6GQTUB5OU4Y7B2FD4RVXADAVCNFSM6AAAAACWYGETT2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSOBSGY2TEMJXG4>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Collaborator
|
The original implementation (in R) only allows 0: parallel to axis, 1:
horiz, 2:perp to axis, and 3: vertical
However, it looks like the METviewer UI does not have the revision series
plot available. Let's keep the commented code for now, in case we plan on
making this available via METviewer.
…On Fri, Mar 20, 2026 at 10:32 AM George McCabe ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In metplotpy/plots/revision_series/revision_series_config.py
<#571 (comment)>:
> @@ -62,11 +62,11 @@ def __init__(self, parameters: dict) -> None:
##############################################
# x-axis parameters
self.x_title_font_size = self.parameters['xlab_size'] + constants.DEFAULT_TITLE_FONTSIZE
- self.x_tickangle = self.parameters['xtlab_orient']
- if self.x_tickangle in constants.XAXIS_ORIENTATION.keys():
- self.x_tickangle = constants.XAXIS_ORIENTATION[self.x_tickangle]
+ # self.x_tickangle = self.parameters['xtlab_orient']
+ # if self.x_tickangle in constants.XAXIS_ORIENTATION.keys():
Thanks for bringing this up. I left this commented because I wasn't sure
how to handle this.
The XAXIS_ORIENTATION constant only maps values to 0 or 270. The plotly
revision_series had the x-tick label orientation hard-coded to 90, so it
was not possible to get the test config to map to the correct orientation.
When I changed this plot to use the _add_xaxis I added to base_plot.py to
handle these consistently, it could not be oriented properly.
XAXIS_ORIENTATION = {0: 0, 1: 0, 2: 270, 3: 270}
I wanted to check why 0-1 map to 0 and 2-3 map to 270 and if that is
correct. Does METviewer set these orientation 0-3 integers?
—
Reply to this email directly, view it on GitHub
<#571?email_source=notifications&email_token=AA4UJHXNZNBCEYYZPNNFXFT4RVXADA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTGOJYGI3DKMRRG432M4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#discussion_r2966758049>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4UJHW6GQTUB5OU4Y7B2FD4RVXADAVCNFSM6AAAAACWYGETT2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSOBSGY2TEMJXG4>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Contributor
Author
|
Edit: I misread the values you shared from the R implementation and it sounds like the current mapping is correct, but doesn't support 90 degree rotation. I will still review this to see if it can be supported better going forward. @bikegeek, OK sounds good. I made a note to review the orientation logic for the next pass of changes after we get feedback from this round. I believe other plots use the dictionary to map 0-3 to a rotation value, so we probably want to get this updated to support all 4 orientations. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Other changes
Differences
Pull Request Testing
Ran unit tests, reviewed image differences (summarized above)
Review code changes
Review image differences
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
Do these changes include sufficient testing updates? [Yes]
Will this PR result in changes to the test suite? [Yes]
If yes, describe the new output and/or changes to the existing output:
Do these changes introduce new SonarQube findings? [No]
If yes, please describe:
Please complete this pull request review by 3/24/2026.
Pull Request Checklist
See the METplus Workflow for details.
Select: Reviewer(s) and Development issue
Select: Milestone as the version that will include these changes
Select: Coordinated METplus-X.Y Support project for bugfix releases or METplotpy-X.Y.Z Development project for official releases