Update cdp_parser.py

[bfiesel]

I was recently working with the arm_diags package that uses this library. I am currently trying to update the arm_diags package from Python 2 to Python 3 respectfully. With the old Python 2 version, we were using cdp V1.0.3 With the updates moving toward Python 3, we are trying to utilize the newest version of cdp v1.6.0. With this, I noticed a potential issue in the code when I tried to use this library in my package. Particularly, on lines 110-120 there is use of __args_namespace.parameter, but this is never used again in the code. It looks like there is not a method that initializes __args_namespace.parameters. Perhaps this was meant to be __args_namespace? If this is the case, I forked this project and made the changes accordingly. If possible I would like to hear your thoughts. Thank you.

[chengzhuzhang]

Thank you @bfiesel !

@zshaheen when you have time, would you please look at this PR? And potentially also help with the py3 conversion PR of arm_diags that is related to apply updated CDP. Thank you!

[durack1]

@chengzhuzhang unfortunately @zshaheen is no longer at LLNL, so it's not clear who is available to assist with this, @gleckler1 may have an idea.

@bfiesel nice avatar!

[chengzhuzhang]

@durack1 hey, thanks! Yeah, i talked to Zeshawn not long ago about possible assistance we need particularly with using updated version of CDP in ARM-diags. He might be able to help on this PR. And I agree, it is a good idea to identify a new POC for CDP for future development.

[zshaheen]

This line checks that the parameters defined in the file, e.g. params.py below, are processed and in this namespace.

e3sm_diags -p path/to/params.py

A namespace was used because when parse_args() is called more than one time, from the second time onwards, it deletes all of the args it read in. This was the case when a user would call view_args(), which would just return the value of parse_args(). Thus, I saved the values of parse_args() into a variable.

[zshaheen]

Looking at the code for CDPParameter.load_parameter_from_py, a path to the file passed in via -p (see my comment above) must be used. So just passing in a Namespace object will not work.

[zshaheen]

Hi @bfiesel. I left some comments on this PR.

With this, I noticed a potential issue in the code when I tried to use this library in my package.

Could you explain the actual issues in your code and why this change was needed? We might be able to help with that.

---

So the __args_namespace variable contains the namespace, which is the output of the Python argparse internal method parse_args. This is needed because calling parse_args multiple times results in an empty value returned. Hence we store the result in __args_namespace.

As per the comments in the code, references to __args_namespace.parameters actually refers to the path of the Python file passed in via the -p/--parameters arg. The value of parameters is set in __args_namespace when load_default_args is called. The actual line that does it is below.

self.add_argument(
    '-p', '--parameters',
    type=str,
    dest='parameters',  # This is the line that sets the value accessible via self.__args_namespace.parameters
    help='Path to the user-defined parameter file.',
    required=False)

[bfiesel]

Hi @zshaheen. Thank you for the response and information regarding this, I found it very helpful. This information helped considerably with my issue. I am trying to fix another error in the arm_diags package right now. I will reach out if I am confused about the cdp library. I believe that my current issue is not linked to this library. Thanks again!

[zshaheen]

Sure, no problem. For more help, I'd take a look at the e3sm_diags package, another piece of software made by the climate department at LLNL. Specifically, this function uses a CDP parser to get the parameters to run the diagnostics. It has more comments.

By following one of the e3sm_diags quickguides (just reading along, figuring out how users interact with the parameters) and playing around with the acme_diags_driver.py file, it'll help with getting a handle of things.

You're doing a great job!