search

E3SM-Project / e3sm_diags

E3SM Diagnostics package
https://e3sm-project.github.io/e3sm_diags
BSD 3-Clause "New" or "Revised" License
38 stars 31 forks source link

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

Open tomvothecoder opened 5 months ago

tomvothecoder commented 5 months ago

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) https://github.com/E3SM-Project/e3sm_diags/blob/43f5f74057929c4bedddfab58be63f838ee36abe/e3sm_diags/derivations/acme.py#L19-L21

  2. Example rename() reference in derived vars dictionary -- the intended behavior is to rename "rsdt" to "SOLIN" https://github.com/E3SM-Project/e3sm_diags/blob/43f5f74057929c4bedddfab58be63f838ee36abe/e3sm_diags/derivations/acme.py#L674-L676

  3. Call to rename() -- notice that a cdms2.TransientVariable is being passed to rename() here. rename() will just return that object without doing anything. https://github.com/E3SM-Project/e3sm_diags/blob/43f5f74057929c4bedddfab58be63f838ee36abe/e3sm_diags/driver/utils/dataset.py#L381-L386

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 commented 5 months ago

@chengzhuzhang FYI