efabless / cace

Circuit Automatic Characterization Engine
https://cace.readthedocs.io/
Apache License 2.0
35 stars 6 forks source link

Use argparse for argument parsing #45

Closed mole99 closed 3 months ago

mole99 commented 3 months ago

This PR simplifies argument parsing by using the builtin module argparse

This is in preparation for the multiprocessing rewrite.

RTimothyEdwards commented 3 months ago

Testing this: I immediately came up with the following error when running on sky130_ef_ip__instramp in cace-gui: "Error: No parameter named I found!" I tracked this down to cace_cli.py line 470 "for paramname in paramnames". There is no such expectation in the old code that "paramnames" would be a list; it is only a string (the parameter name) or None. In the original code, the argument was "paramname", not "paramnames".

If there is a new use case somewhere with a number of parameters being passed as a list, then you at least need above line 470:

        # In case only one parameter is passed in paramnames, convert to a list
        if isinstance(paramnames, str):
            paramnames = [paramnames]

Apart from that, it looks good. If you make that change, you can go ahead and merge it.

mole99 commented 3 months ago

I fixed the bug by only ever passing a list to paramnames.

I have also updated the documentation with the new argument style.