Conversation
|
An initial comment I have on this update is to make the default param names more descriptive. Specifically, change |
Thanks @lizziel, I can do this. |
|
@lizziel @msulprizio: I changed the names of the constants and pushed a couple more fixes. |
3837265 to
c99ef74
Compare
|
@msulprizio @lizziel: This PR has been rebased atop PR #377 (stretched-grid fixes) and #375, and #376 (pypdf==6.0.0) |
f014a2b to
07f3a9a
Compare
In gcpy/constants.py - Renamed "skip_these_vars" to "SKIP_THESE_VARS", as constants should use all caps - Added DEFAULT_LL_EXTENT = [-180, 180, -90, 90] - Added DEFAULT_SG_PARAMS = [1, 170, -90] In various routines - Replaced "skip_these_vars" with "SKIP_THESE_VARS" - Replaced "[-180, 180, -90, 90]" with DEFAULT_LL_EXTENT - Replaced "[-180, 180, -90, 90]" with DEFAULT_SG_PARAMS CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/constants.py - Added DEFAULT_SG_STRETCH_FACTOR, DEFAULT_SG_TARGET_LON, and DEFAULT_SG_TARGET_LAT - Now define DEFAULT_SG_PARAMS with the other DEFAULT_SG_* constants - Updated comments CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/grid.py - Added a shebang line at top of the module - Imported DEFAULT_LL_EXTENT and DEFAULT_SG_PARAMS from constants.py - Edited lines to not be longer than 100 chars where expedient - Removed "else" statements after a "return", they're redundant, and re-indented code accordingly - Removed lists as default values for keywords; Use None instead, then if the keyword is None, set to its appropriate default value - Added PyDoc headers that were missing - Renamed routines to conform to Python snake-case name convention: - make_grid_LL -> make_grid_ll - make_grid_CS -> make_grid_cs - make_grid_SG -> make_grid_sg - csgrid_GMAO -> csgrid_gmao - rotate_sphere_3D -> rotate_sphere_3d - Renamed several variables to conform to Python snake-case name convention - Removed parentheses after equal signs as directed by Pylint - Added cosmetic changes (comments, line breaks, indentations) - Added an else block in routine rotate_sphere_3d to raise a ValueError if the rot_axis is not either "x", "y", or "z" Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/regrid.py - Added the shebang line at the top of the module - Adjusted import statements to import renamed functions in grid.py - Renamed the following functions to better conform to Python snake-case naming convention: - make_regridder_l2l --> make_regridder_ll2ll - make_regridder_C2L --> make_regridder_cs2ll - make_regridder_S2S --> make_regridder_sg2sg - make_regridder_l2S --> make_regridder_ll2sg - Renamed loop variable "regridder" to "the_regridder" to avoid having a name clash with an argument named "regridder" - Edited code to make sure lines of code are less than 100 chars. - Converted strings using ".format()" to f-strings - Modified code to no longer trap for BaseException - Modified code so that lists are not used as default values for keyword arguments. Use None instead and then if the keyword is None, then set it to default values within the program. - Renamed local variables to conform to snake-case convention - Cosmetic changes (comments, indentation, trimmed whitespace) Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/cstools.py - Updated code to import and use the renamed "csgrid_gmao" function from "gcpy/grid.py" - Modified the assigment statement for x_lat to use "i_vert in (0, 3)" - Fixed incorrect indentation at line 753 CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/file_regrid.py - Now imports and uses the make_regridder_ll2ll, make_regrider_cs2ll, make_regridder_sg2sg, and make_regridder_ll2sg functions that have been renamed in gcpy/grid.py. Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/append_grid_corners.py - Import DEFAULT_SG_PARAMS from constants.py and use that instead of hardwired [1, 170, -90] - Updated code to use renamed "make_grid_sg" instead of the older "make_grid_SG" function (from grid.py) CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/raveller_1D.py - Now import the renamed make_grid_cs from gcpy/grid.py - Added missing PyDoc headers - Break lines up so they do no excceed 80 chars - Cosmetic changes for readability Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/constants.py - Renamed DEFAULT_LL_EXTENT -> GLOBAL_LL_EXTENT - Renamed DEFAULT_SG_* -> NO_STRETCH_SG_* CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/grid.py - Now import newly-renamed GLOBAL_LL_EXTENT constant - Now import newly-renamed NO_STRETCH_SG_* constants - Renamed vert_grid class to VertGrid, as Python classes should use PascalCase naming convention
gcpy/append_grid_corners.py gcpy/regrid.py - Now import newly-renamed GLOBAL_LL_EXTENT constant - Now import newly-renamed NO_STRETCH_SG_* constants Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/cstools.py - Now call gcpy.make_grid_sg instead of gcpy.make_grid_SG. This should have been changed in a prior commit but wasn't. Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/regrid.py - The CodeQL GitHub Action flagged the use of hashlib.sha1 as being cryptographically weak. We have adopted the recommendation of using hashlib.sha256 instead. CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
grid.py regrid.py - Renamed the last remaining instances of vert_grid to VertGrid. These were missed in the previous commit. Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/plot/compare_single_level.py - Replaced 2 additional instances of [1,170,-90] with the constant NO_STRETCH_SG_PARAMS from gcpy/constants.py. gcpy/plot/compare_zonal_mean.py - Replaced 2 additional instances of [1,170,-90] with the constant NO_STRETCH_SG_PARAMS from gcpy/constants.py. The instances of [1,170,-90] were introduced from a rebase. Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
9ced073 to
9654a75
Compare
gcpy/grid.py - Import GLOBAL_LL_EXTENT, NO_STRETCH_SG_PARAMS, and R_EARTH_m from gcpy.constants, not gcpy.vgrid_defs - Change calls to "vert_grid" to "VertGrid", to conform to PascalCase naming convention for Python classes Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
This merge brings updates from the "dev" branch (as of PR #381) back into the "feature/update-constants" branch. Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
|
I have merged the |
|
Thanks @yantosca, I will do a re-review this week. |
CHANGELOG.md
Outdated
| - Renamed `skip_these_vars` to `SKIP_THESE_VARS` to conform to Python style for constants | ||
| - Modified code in `grid.py`, `regrid.py`, `file_regrid.py`, and `cstools.py`, and `raveller_1D.py` as directed by Pylint to better conform to the Python style guide | ||
| - Renamed the following routines in `gcpy/grid.py` to conform to snake-case naming convention | ||
| - `csgrid_GMAO --> csgrid_gmao` |
There was a problem hiding this comment.
I think this level of detail in the changelog is a bit too much. Best keep it simple, e.g. "Renamed grid routines to use all lower-case to conform to snake-case naming convention". Same comment for other sections of changelog that have bulleted lists.
Replaced explicit changes with more general descriptions Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/plot/compare_single_level.py gcpy/plot/compare_zonal_mean.py - Import NO_STRETCH_SG_PARAMS from gcpy.constants. This had been omitted in PR #374. Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/examples/grids/display_gcclassic_grid_info.py - Rebased atop PR #374; use consistent snake_case format - Added the type of grid ("global", "nested NA", etc") as the 4th element in the grid_parameters tuple - Now display metadata (Name, Resolution, Longitude range, Latitude Range) before displaying the lon & lat centers & edges - Add "res_display" variable to contain a "pretty" version of the resolution string (with degree symbols) Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
gcpy/examples/grids/display_gcclassic_grid_info.py - Rebased atop PR #374; use consistent snake_case format - Added the type of grid ("global", "nested NA", etc") as the 4th element in the grid_parameters tuple - Now display metadata (Name, Resolution, Longitude range, Latitude Range) before displaying the lon & lat centers & edges - Add "res_display" variable to contain a "pretty" version of the resolution string (with degree symbols) Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
This merge brings PR #396 (Allow args to create_regridders to be xr.Dataset or xr.DataArray, by @yantosca) into the GCPy 1.7.0 development stream. PR #396 fixes a bug introduced in PR #374, where an error was thrown if arguments to the "create_regridders" function were not of type xarray.Dataset. We now also allow arguments of type xarray.DataArray. Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
Name and Institution (Required)
Name: Bob Yantosca
Institution: Harvard + GCST
Describe the update
This PR does the following:
Renames the constant
skip_these_varstoSKIP_THESE_VARS(as constants in Python should be upper case), and replaced them in various routines.Adds
GLOBAL_LL_EXTENT,DEFAULT_SG_STRETCH_FACTOR,DEFAULT_SG_TARGET_LAT,DEFAULT_SG_TARGET_LON, andNO_STRETCH_SG_PARAMStogcpy/constants.py.[-180, 180, -90, 90]withGLOBAL_LL_EXTENTin various routines[1, 170, -90]withNO_STRETCH_SG_PARAMSin various routinesAdd modifications suggested by Pylint to
gcpy/grid.py:GLOBAL_LL_EXTENTandNO_STRETCH_SG_PARAMSfromconstants.pyAdd modifications suggested by Pylint to
gcpy/regrid.pygrid.pyregriddertothe_regridderto avoid having a name clash with an argument named "regridder".format()to f-stringsBaseExceptionAdd modifications suggested by Pylint to
gcpy/cstools.pycsgrid_gmaofunction fromgcpy/grid.pyx_latto usei_vert in (0, 3)Updated
gcpy/file_regrid.pyto import the renamedmake_regridder_*functions fromgcpy/grid.pyUpdate
gcpy/append_grid_corners.pyas follows:[1, 170, -90]Update
gcpy/raveller_1D.pyas followsmake_grid_csfromgcpy/grid.pyExpected changes
This should be a zero-diff change.
Related Github Issue
N/A