Skip to content

b4b-dev: Python scripts: Require user to specify lon format when ambiguous #3024

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

Merged
merged 32 commits into from
Apr 16, 2025

Conversation

samsrabin
Copy link
Member

@samsrabin samsrabin commented Mar 21, 2025

Description of changes

At the moment, our Python tools are dangerous in that they can allow the user to enter longitude in the [-180, 180] format when the tools actually assume the [0, 360] format. This PR fixes that by, in ambiguous cases, requiring the user to specify the format they're using with --lon-type either 180 or 360.

Specific notes

Contributors other than yourself, if any: None

CTSM Issues Fixed:

Are answers expected to change (and if so in what way)? No

Any User Interface Changes (namelist or namelist defaults changes)? No

Does this create a need to change or add documentation? Did you do so? Yes; see ESCOMP/ctsm-docs#9.

Testing performed, if any:
As of b97904f:

  • Python unit and system tests passed but should be rerun before merge.
  • Docs build OK (only pre-existing issues).
  • clm_pymods test suite

@samsrabin samsrabin changed the title [WIPFix python tools longitude format [WIP] Fix python tools longitude format Mar 21, 2025
@samsrabin samsrabin self-assigned this Mar 21, 2025
@samsrabin
Copy link
Member Author

samsrabin commented Apr 1, 2025

  • Done with e5bbb14: If all longitudes are unambiguous, assume longitude format

* Require that user specify whether longitude is in [-180, 180] format (i.e., centered around Prime Meridian) or [0, 360] format (i.e., centered around International Date Line).
* Specified as --lon-type, either 180 or 360 (respectively)
* Resolves ESCOMP#2017
* Resolves ESCOMP#3001
@samsrabin samsrabin force-pushed the fix-python-tools-longitude-format branch from dd68889 to 6b29b53 Compare April 3, 2025 19:47
@samsrabin samsrabin changed the base branch from master to b4b-dev April 3, 2025 19:47
@samsrabin samsrabin added bug something is working incorrectly bfb bit-for-bit test: python Pass clm_pymods test suite plus Python sys/unit tests before merging labels Apr 3, 2025
@github-project-automation github-project-automation bot moved this to Ready to start (or start again) in CTSM: Upcoming tags Apr 3, 2025
@samsrabin samsrabin moved this from Ready to start (or start again) to In progress - master/b4b-dev in CTSM: Upcoming tags Apr 3, 2025
@samsrabin samsrabin linked an issue Apr 3, 2025 that may be closed by this pull request
@samsrabin samsrabin marked this pull request as ready for review April 4, 2025 23:36
@samsrabin samsrabin changed the title [WIP] Fix python tools longitude format Fix python tools longitude format Apr 4, 2025
@samsrabin samsrabin added PR status: awaiting review Work on this PR is paused while waiting for review. next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Apr 4, 2025
@samsrabin samsrabin changed the title Fix python tools longitude format Python scripts: Require user to specify lon format when ambiguous Apr 4, 2025
@samsrabin samsrabin added documentation additions or edits to user-facing documentation test: docs Test documentation build before merging labels Apr 4, 2025
@samsrabin samsrabin requested a review from ekluzek April 8, 2025 21:35
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

Cool, this is great, and makes everything so much more robust.

I do have both a few things I'd like you to do, and some suggestions on things you can decide on. But, I'm marking as Approve, so you don't have to track me down again.

I feel the strongest about removing using of "International Date Line" for something like 180m for 180 Meridian. And I have a few possible suggestions to _detect_lon_type, where I think pulling in something there would be good. But, leaving it up to you to decide on this.

@ekluzek ekluzek changed the title Python scripts: Require user to specify lon format when ambiguous b4b-dev: Python scripts: Require user to specify lon format when ambiguous Apr 14, 2025
@samsrabin samsrabin removed next this should get some attention in the next week or two. Normally each Thursday SE meeting. PR status: awaiting review Work on this PR is paused while waiting for review. labels Apr 15, 2025
@samsrabin samsrabin force-pushed the fix-python-tools-longitude-format branch from 42a03b4 to c64bc2e Compare April 15, 2025 18:56
@samsrabin samsrabin merged commit 83935b9 into ESCOMP:b4b-dev Apr 16, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from In progress - master/b4b-dev to Done (non release/external) in CTSM: Upcoming tags Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit bug something is working incorrectly documentation additions or edits to user-facing documentation test: docs Test documentation build before merging test: python Pass clm_pymods test suite plus Python sys/unit tests before merging
Projects
Status: Done (non release/external)
Development

Successfully merging this pull request may close these issues.

Make Python scripts smarter about lons [-180, 180) vs. [0, 360)
2 participants