NREL / disco

DISCO
BSD 3-Clause "New" or "Revised" License
10 stars 5 forks source link

Minor OpenDSSDirect.py usage issues #153

Closed PMeira closed 2 years ago

PMeira commented 2 years ago

Hi, all, just a heads up for a couple of minor issues.

  1. Indirectly through an issue opened on ODD.py, I noticed that the filename needs to be enclosed in quotes or brackets here:

https://github.com/NREL/disco/blob/f32b34026a6496b413c264598869493cb3a1565a/disco/extensions/upgrade_simulation/upgrades/common_functions.py#L61

  1. And since the commands used in DISCO are all single-line commands, I'd also recommend replacing dss.run_command(...) with dss.Text.Command(...) so the errors are mapped to exceptions directly, here:

https://github.com/NREL/disco/blob/f32b34026a6496b413c264598869493cb3a1565a/disco/extensions/upgrade_simulation/upgrades/common_functions.py#L1268

and here:

https://github.com/NREL/disco/blob/f32b34026a6496b413c264598869493cb3a1565a/disco/cli/compute_pv_swad.py#L92

daniel-thom commented 2 years ago

@PMeira Thanks for the notes. I didn't know about dss.Text.Command.

Regarding the problem about quotes or brackets, this actually works fine. Are you saying that it is supposed to be dss.run_command("Redirect 'Master.dss'")?

daniel-thom commented 2 years ago

Also @PMeira, this is on the opendssdirect.py documentation "Getting Started" page. Maybe it should specify dss.Text.Command?

from opendssdirect.utils import run_command
run_command('Redirect ../../tests/data/13Bus/IEEE13Nodeckt.dss');
PMeira commented 2 years ago

Regarding the problem about quotes or brackets, this actually works fine. Are you saying that it is supposed to be dss.run_command("Redirect 'Master.dss'")?

@daniel-thom If the path is guaranteed to have no special chars (as seen by the DSS engine) and spaces, then it's fine indeed. Otherwise, if something like this is possible "Redirect /home/test/test 1/Master.dss", the filename will be interpreted as /home/test/test instead (the rest is currently discarded following the official OpenDSS behavior).

Also @PMeira, this is on the opendssdirect.py documentation "Getting Started" page. Maybe it should specify dss.Text.Command?

Yep, I'd say that needs to be updated (related: https://github.com/dss-extensions/OpenDSSDirect.py/issues/70), thanks for pointing that out.

Maybe @kdheepak would remember why exactly run_command exists, but I believe it's a helper function in case you needed to run multiple commands and get the output for each line. That is useful for interactive usage (in notebooks, for example), but not the best in general for automated runs. Since we added the mapping of DSS error code to exceptions a few years back, we probably should have deprecated or adjusted run_command then.