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

Update gcpy_env environment to install with MambaForge and to use latest package versions; Update scripts accordingly #251

Merged
merged 14 commits into from
Aug 17, 2023

Conversation

yantosca
Copy link
Contributor

In this PR we have done the following:

  1. Updated the docs/source/environment.yml file as follows:

    • Use conda-forge and nodefaults channels
    • Remove all installations via pip
    • Remove most version numbers; let MambaForge find dependencies, except for:
      • xesmf==0.5.1, with esmf==8.1.1 and esmpy==8.1.1, because newer versions break backward compatibility.
      • sphinx==3.5.4 and related dependencies, in order to ensure that ReadTheDocs documentation builds properly
    • Added gridspec (which is needed for regridding with gridspec and sparselt)
  2. Removed the separate docs/source/gchp_regridding.yml environment file.

  3. Updated routines in plot.py and util.py for compatibility with newer package versions:

    • PyPDF2 is now pypdf; several function calls had to be modified.
    • matplotlib was updated from 3.4.2 to 3.7.2; several modifications were necessary:
      • matplotib.colors.RdBu_r --> matplotlib.colormaps["RdBu_r"]
      • matplotlib.colorbar.set_ticklabels -> matplotlib.colorbar.set_ticks with the labels argument.
    • Added coding style updates suggested by pylint.
    • Trimmed whitespace, updated comments.
  4. Abstracted and "never-nested" code from routine six_plot (inplot.py) into separate functions, for clarity.

A separate documentation update will be pushed to the docs/dev branch.

docs/environment_files/environment.yml
- Move all pip dependencies to the conda/mamba section, as there are
  now some issues in resolving dependencies with pip.

Signed-off-by: Bob Yantosca <[email protected]>
docs/environment_files/environment.yml
- Remove most pegged version numbers, which cause mamba/conda
  installations to fail with conflicts.

Signed-off-by: Bob Yantosca <[email protected]>
docs/environment_files/environment.yml
- Need to peg esmf==8.1.1, esmpy==8.1.1, xesmf==0.5.1
  to prevent an ImportError

gcpy/plot.py
- Now import PdfFileMerger from the pypdf package

gcpy/util.py
- Now import PdfFileWriter, PdfFileReader from the pypdf package

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/plot.py
- PdfFileMerger is now PdfMerger
- Now use None as default values.  This avoids dangerous empty lists
  ([]) as default values for keyword arguments (so says Pylint).
- Now use util.verify_variable_type to error-check arguments
- Now update indendation
- Trim trailing whitespace
- Updated Pydoc comments

gcpy/util.py
- PdfFileWriter is now PdfWriter
- PdfFileReader is now PdfReader
- Remove "overwriteWarnings=False" from PdfReader, it's not used
- PdfWriter.addPage is now PdfWriter.add_page
- PdfWriter.addBookmark is now PdfWriter.add_outline_item
- PdfWriter.setPageMode is now PdfWriter.page_mode

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/plot.py
- In routine single_panel,
  - Set norm=[] if it is passed in as None
    (typo: we had set sigdiff_list=[])
  - In the "if xtick_positions is None:" block, now set
    xtick_positions=[] by default.  This fixes a "NoneType object
    is not iterable" error when plotting maps.
  - Now use "except ____ as exc:", which is now the recommended format

Signed-off-by: Bob Yantosca <[email protected]>
docs/environment_files/environment.yml
- Added gridspec to the GCPy Python environment file, which allows
  us to perform regridding with gridspec/sparselt

docs/environment_files/gchp_regridding.yml
- Removed

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/plot.py
- In routine "compare_single_level",
  - Set extent=None in the argument list.  Then at the top of the
    if extent is None, set it to [-1000, -1000, -1000, -1000].
    (Suggested by Pylint)
  - diff_of_diffs is now computed with a boolean test instead of
    an if-else statement (suggested by Pylint)
  - Comment out ref_extent and dev_extent, as Pylint detected that
    these variables were unused.
  - Fix "line-too-long" errors where ds_ref_reg and ds_dev_reg are
    computed (suggested by Pylint).
  - Comment out vmin_ref_cmp and vmin_dev_cmp, as these were detected
    as unused by Pylint
  - Pass absdiff.values to all_zero_or_nan (error detected by Pylint)
  - Replace mpl.cm.RdBu_r with mpl.colormaps["RdBu_r"], this is the new
    syntax in the most recent matplotlib version

- In routine compare_zonal_mean:
  - Set pres_range=None in the argument list, and add a test to set
    pres_range=[0, 2000] if pres_range is None (suggested by Pylint)
  - diff_of_diffs is now computed with a boolean test instead of
    an if-else statement (suggested by Pylint)
  - Replace mpl.cm.RdBu_r with mpl.colormaps["RdBu_r"], this is the new
    syntax in the most recent matplotlib version
  - Use isinstance instead of direct type comparison

- In routine single_panel:
  - Set pres_range=None in the argument list, and add a test to set
    pres_range=[0, 2000] if pres_range is None (suggested by Pylint)
  - Remove the "if norm is None:" block at the top of the routine,
    since a similar test is done further down.  This had reset the
    norm object from None to [], which caused an if block further
    down to not execute properly.  This caused colorbar tickmarks
    to render incorrectly.
  - Replace "except BaseException as exc" as "except BaseException"
    (suggested by Pylint)
  - Also restore extent=(None, None, None, None) to the if block
    that tests if extent was passed.  But put this after the
    "Extent is None" clause since that is now the default.
  - Comment out Pydoc string
  - Replace strings with f-strings where expedient
  - Remove else after return (suggested by Pylint)

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/constants.py
- Add ENCODING = "UTF-8", for use in open statements

gcpy/util.py
- Added code change suggested by Pylint:
  - Variable names now conform to snake_case (i.e. more than 3 chars,
    and separated by "_"), in particular:
    - "v" -> "var"
    - "ds" -> "dset"
    - "dr" -> "darr"
    - "f" -> "ifile" or "ofile"
  - open statements are now placed in with blocks
  - open statements now use encoding=ENCODING (aka UTF-8)
  - In routine convert_lon, changed "format" to "fmt" to avoid
    clashing with a Python reserved word
  - Remove () in if blocks, they aren't necessary
  - Trimmed trailing whitespace
  - Remove else blocks with return statements (we de-indent instead)
- Also use verify_variable_type to check argument types

CHANGELOG.md
- Updated accordingly

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/plot.py
- In routine compare_single_level, use argument absdiff instead of
  absdiff.values when calling all_zero_or_nan.  This is because absdiff
  is already a numpy.ndarray and not an xarray.DataArray object.

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/plot.py
- Move code that computes vmin, vmax out of "six_plot" and into new
  routine compute_vmin_vmax_for_plot.  Replaced else statements by
  "never-nesting".
- Move code that computes the "norm" object out of "six_plot" and
  into new routine "compute_norm_for_plot".
- Move code that applies colorbar tick placement and formatting out
  of "six_plot" and into new routine "colorbar_ticks_and_format".
  Also tweaked settings so that for data ranges of between 0.1 and 100,
  we manually set colorbar ticks.
- Move the declaration of the "subplots" list from "compare_single_level"
  and "compare_zonal_mean" to a new routine "six_panel_subplot_names".
- Add if statements to disable parallelization if n_job=1.
- Added code formatting updates suggested by Pylint, where expedient
- Updated comments & PyDoc headers
- Trimmed trailing whitespace

CHANGELOG.md
- Updated accordingly

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/plot.py
- In routine "colorbar_ticks_and_format", now use the tick label
  locations [vmin, (vmin+1)/2, 1, (vmax+1)/2, vmax] for ratio
  (dynamic range) subplots (if the data is in the range 0.1..100).
  This fixes an error with the colorbar tick labels from the previous
  commit.

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/plot.py
- In routine "colorbar_ticks_and_format", add an if block to
  add the "Ref and Dev are equal throughout domain" to the
  absdiff plots.  This was omitted during the "never-nesting"
  refactoring.
- Remove unnecessary parentheses in if block
- Update comments

Signed-off-by: Bob Yantosca <[email protected]>
gcpy/plot.py
- In routine six_plot:
  - Remove calls to cbar.minorticks_off() and cbar_update_ticks()
- In routine colorbar_ticks_and_format:
  - Update comments
  - Change "return" commands to "return cbar" in order to make sure
    that the modified colorbar settings get passed back.
  - Now apply cbar.set_ticks with position list and labels list.
  - Also apply cbar.minorticks_off() after each call to cbar.set_ticks().
  - Comment out max_abs variable, Pylint says it is unused

Signed-off-by: Bob Yantosca <[email protected]>
@yantosca yantosca added category: Feature Request New feature or request topic: User Environment Relating to python and/or conda environment topic: Benchmark Plots and Tables Issues pertaining to generating plots/tables from benchmark output labels Aug 17, 2023
@yantosca yantosca requested a review from laestrada August 17, 2023 18:04
@yantosca yantosca self-assigned this Aug 17, 2023
@yantosca yantosca added this to the 1.4.0 milestone Aug 17, 2023
Copy link
Contributor

@laestrada laestrada left a comment

Choose a reason for hiding this comment

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

Looks good to me. I just built and pushed the container to use the new environment, so you are all set to merge.

gcpy/plot.py
- In routine colorbar_ticks_and_format, add a new internal function
  "ref_equals_dev" that returns True if Dev/Ref = 1.  Also returns
  true if there are missing (NaN) values, as long as all other values
  are 1.  This is necessary to make sure the "Ref and Dev equal throughout
  domain" label gets printed in ratio plots.

Signed-off-by: Bob Yantosca <[email protected]>
Copy link
Contributor Author

Thanks @laestrada! Will do.

@yantosca yantosca merged commit 1a75e77 into dev Aug 17, 2023
@yantosca yantosca deleted the feature/mamba-env-install branch August 17, 2023 20:20
yantosca added a commit that referenced this pull request Aug 17, 2023
This merge brings updates from PR #251 (Update gcpy_env environment
to install with MambaForge and to use latest package versions; Update
scripts accordingly, by @yantosca) into the feature/move-folders branch.

This will facilitate merging into dev at a later time.

Signed-off-by: Bob Yantosca <[email protected]>
yantosca added a commit that referenced this pull request Aug 17, 2023
This merge brings updates from PR #251 (Update gcpy_env environment
to install with MambaForge and to use latest package versions; Update
scripts accordingly, by @yantosca) into the
feature/disable-parallelization branch.

This will facilitate merging into dev at a later time.

Signed-off-by: Bob Yantosca <[email protected]>

--
hannahnesser pushed a commit to hannahnesser/gcpy that referenced this pull request Jan 22, 2024
…mbaForge)

This merge brings PR geoschem#251 (Update gcpy_env environment to install with
MambaForge and to use latest package versions; Update scripts accordingly,
by @yantosca) into the GCPy 1.4.0 development stream.

This updates the docs/environments/environment.yml file to use more
modern package versions, thus eliminating the dependence on Conda 4.12,
and allowing installation with MambaForge.  Several updates were also
made to gcpy/util.py and gcpy/plot.py due to changes in Python packages.

Furthermore, we have now abstracted some code out of the six_plot routine
in gcpy/plot.py into separate functions. This should make the code
easier to maintain and understand.

See geoschem#251 for more information.

Signed-off-by: Bob Yantosca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Feature Request New feature or request topic: Benchmark Plots and Tables Issues pertaining to generating plots/tables from benchmark output topic: User Environment Relating to python and/or conda environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants