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

[Bug]: main branch not properly renaming original variables to target derived variable key #796

Open
tomvothecoder opened this issue Mar 4, 2024 · 1 comment
Labels
bug Bug fix (will increment patch version)

Comments

@tomvothecoder
Copy link
Collaborator

tomvothecoder commented Mar 4, 2024

What happened?

rename() is not properly renaming original variable names to target derived variable keys. This bug has not been causing any issues in e3sm_diags and we don't write out files to netCDF (parameter.save_to_netcdf=False by default). This is a low priority issue and can be addressed when desired.

Technical Info

  1. rename() -- notice how it just returns whatever is passed to it (new_name)

    def rename(new_name):
    """Given the new name, just return it."""
    return new_name

  2. Example rename() reference in derived vars dictionary -- the intended behavior is to rename "rsdt" to "SOLIN"

    "SOLIN": OrderedDict(
    [
    (("rsdt",), rename),

  3. Call to rename() -- notice that a cdms2.TransientVariable is being passed to rename() here. rename() will just return that object without doing anything.

    # Get the corresponding function.
    # Ex: The func in {('PRECC', 'PRECL'): func}.
    func = self._get_func(vars_to_func_dict)
    # Call the function with the variables.
    derived_var = func(*variables)

What did you expect to happen? Are there are possible answers you came across?

rename() should update the .id of the cdms2.TransientVariable to the target key

Example:

def rename(var: cdms2.TransientVariable, target_key: str) -> cdms2.TransientVariable:
    var.id = target_key

    return var

Minimal Complete Verifiable Example (MVCE)

No response

Relevant log output

No response

Anything else we need to know?

No response

Environment

main branch and all previous versions of e3sm_diags that used rename()

@tomvothecoder tomvothecoder added the bug Bug fix (will increment patch version) label Mar 4, 2024
@tomvothecoder
Copy link
Collaborator Author

@chengzhuzhang FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug fix (will increment patch version)
Projects
None yet
Development

No branches or pull requests

1 participant