Skip to content

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

Merged
samsrabin merged 32 commits intoESCOMP:b4b-devfrom
samsrabin:fix-python-tools-longitude-format
Apr 16, 2025
Merged

b4b-dev: Python scripts: Require user to specify lon format when ambiguous#3024
samsrabin merged 32 commits intoESCOMP:b4b-devfrom
samsrabin:fix-python-tools-longitude-format

Conversation

@samsrabin
Copy link
Copy Markdown
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
Copy Markdown
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 b4b 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
Comment thread python/ctsm/subset_data.py Outdated
Comment thread python/ctsm/subset_data.py Outdated
@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 or its infrastructure test: docs Test documentation build before merging labels Apr 4, 2025
Comment thread python/ctsm/subset_data.py Outdated
@samsrabin samsrabin requested a review from ekluzek April 8, 2025 21:35
Copy link
Copy Markdown
Contributor

@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.

Comment thread python/ctsm/config_utils.py Outdated
Comment thread doc/source/users_guide/running-single-points/running-single-point-subset-data.rst Outdated
Comment thread python/ctsm/crop_calendars/cropcal_utils.py Outdated
Comment thread python/ctsm/config_utils.py Outdated
Comment thread python/ctsm/modify_input_files/modify_mesh_mask.py Outdated
Comment thread python/ctsm/subset_data.py
Comment thread python/ctsm/subset_data.py Outdated
@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

b4b bit-for-bit bug something is working incorrectly documentation additions or edits to user-facing documentation or its infrastructure 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)

3 participants