Refactor lib.py utilities to reduce code duplication and excessive nesting of function calls
#190
Assignees
Labels
refactor
Clean up or optimize code
Comments
Closed
tomvothecoder
changed the title
lib.py utilities to reduce code duplicationlib.py utilities to reduce code duplication and excessive nesting of function calls
tomvothecoder
added a commit
that referenced
this issue
Nov 8, 2023
1. Refactor `lib.py` (now deleted) - Closes #190 - Replace `run_parallel()` and `run_serial()` with `__main__.py` -> `E3SMtoCMIP._run_parallel()` and `E3SMtoCMIP._run_serial()` - Updated var name `filepaths` to `vars_to_filepaths` for clarity (its a dict mapping var key to list of string paths) - Update args passed to handler method, remove unnecessary args - Add `try` and `except` statement for submitting `pool` jobs to maintain compatibility with MPAS variable handlers, which use different handler method arguments - Replace `handle_variables()`, `get_dimension_data()` and `load_axis()` with `VarHandler.cmorize()` - Delete `handle_simple()` -- _will be re-implemented from scratch in #130_ 3. Update `handler.py` - Update `VarHandler.cmorize()` - Refactored significantly -- extracted logic into smaller, maintainable private methods - Replaced `data` dictionary storing `xr.DataArrays` with `ds` (`xr.Dataset` object) - Distinguish CMOR usage with "cmor" string in function names and python variables - Add support for hybrid sigma variables: `pfull`, `phalf` - Add `VarHandler._cmor_write()` - Add support for CMORizing fixed-time variables - Separate PR for #217 - If time dimension exists, CMORize the variable with all time and time bound values with a single call to `cmor.write()` instead of looping over each time value index and CMORizing each slice -- **this should improve performance and removes the `tqdm` progress bar.** 4. Update `handlers.yaml` - Add `phalf` and `pfull` entries - Closes #115 - Delete `phalf.py` and `pfull.py` - Add `clcalispo` entry - Closes #218 - Delete `clcalipso.py` 5. Update `_formulas.py` - Update all function argument types and return types from `np.ndarray` to `xr.DataArray` - Add formula functions for `pfull` and `phalf` - Add `convert_units()` function, which handles 1-to-1 unit conversions -- replaces `default.default_handlers.write_data()` 6. Refactor `default.py` (now deleted) - Replaced by `VarHandler.cmorize()` and `_formulas.py.convert_units()` 7. Remove `cdms2` and `cdutil` from dependencies in `dev.yml` and `ci.yml`7 8. Clean up legacy code in `clisccp.py` - Separate PR for #218 9. Add `Makefile` for easy access to commonly used commands (e.g., building and installing package)
tomvothecoder
added a commit
that referenced
this issue
Nov 10, 2023
…atm handlers (#213) 1. Refactor `lib.py` (now deleted) - Closes #190 - Replace `run_parallel()` and `run_serial()` with `__main__.py` -> `E3SMtoCMIP._run_parallel()` and `E3SMtoCMIP._run_serial()` - Updated var name `filepaths` to `vars_to_filepaths` for clarity (its a dict mapping var key to list of string paths) - Update args passed to handler method, remove unnecessary args - Add `try` and `except` statement for submitting `pool` jobs to maintain compatibility with MPAS variable handlers, which use different handler method arguments - Replace `handle_variables()`, `get_dimension_data()` and `load_axis()` with `VarHandler.cmorize()` - Delete `handle_simple()` -- _will be re-implemented from scratch in #130_ 3. Update `handler.py` - Update `VarHandler.cmorize()` - Refactored significantly -- extracted logic into smaller, maintainable private methods - Replaced `data` dictionary storing `xr.DataArrays` with `ds` (`xr.Dataset` object) - Distinguish CMOR usage with "cmor" string in function names and python variables - Add support for hybrid sigma variables: `pfull`, `phalf` - Add `VarHandler._cmor_write()` - Add support for CMORizing fixed-time variables - Separate PR for #217 - If time dimension exists, CMORize the variable with all time and time bound values with a single call to `cmor.write()` instead of looping over each time value index and CMORizing each slice -- **this should improve performance and removes the `tqdm` progress bar.** 4. Update `handlers.yaml` - Add `phalf` and `pfull` entries - Closes #115 - Delete `phalf.py` and `pfull.py` - Add `clcalispo` entry - Closes #218 - Delete `clcalipso.py` 5. Update `_formulas.py` - Update all function argument types and return types from `np.ndarray` to `xr.DataArray` - Add formula functions for `pfull` and `phalf` - Add `convert_units()` function, which handles 1-to-1 unit conversions -- replaces `default.default_handlers.write_data()` 6. Refactor `default.py` (now deleted) - Replaced by `VarHandler.cmorize()` and `_formulas.py.convert_units()` 7. Remove `cdms2` and `cdutil` from dependencies in `dev.yml` and `ci.yml`7 8. Clean up legacy code in `clisccp.py` - Separate PR for #218 9. Add `Makefile` for easy access to commonly used commands (e.g., building and installing package)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Many utilities within
lib.pycontain duplicated code (conditionals, for loops, etc.). Refactoring these utilities can make it easier to extend functionality and fix bugs as needed. The script is lengthy and not well documented, thus is not easy to modify and maintain by developers.Modules to refactor
handle_variables()VarHandlerclass (cmorizemethod wraps this function right now)handle_simple()ifsimple=True(this should be a separate function call)handle_variables()via private functionsThe text was updated successfully, but these errors were encountered: