search

dss-extensions / OpenDSSDirect.py

OpenDSSDirect.py: a cross-platform Python package that implements a native/direct library interface to the alternative OpenDSS engine from DSS-Extensions.org
https://dss-extensions.org/OpenDSSDirect.py/
Other
87 stars 21 forks source link

Deprecate `run_command` #70

Closed kdheepak closed 8 months ago

kdheepak commented 4 years ago

run_command returns an empty string on success and returns a string with the error on failure. Users using run_command in large applications have silent failures at the moment that are hard to debug. I'm opening this issue to discuss deprecating run_command and encouraging users to use API functions first and fallback to a run_command equivalent when the API does not exist.

julioolvera1 commented 4 years ago

Hi @kdheepak ,

I think there are commands that need the run_command. For instance set LoadMult=X.

I was just trying the LoadMult command using run_command() however when using a loadshape it doesn't seem to affect the loads. Also, like you said it doesn't throw any error.

kdheepak commented 4 years ago

Thanks for the comment. I think the fact that run_command exists is resulting in people not asking for features like setting LoadMult=X :)

I know it's extremely valuable to someone coming from using OpenDSS but I've also seen code that has abused this function's existence and also been personally bitten by not checking the return value of run_command. And as you've pointed out, it can also silently fail in unexpected ways.

Perhaps the solution is to deprecate run_command but expose a _run_command that we don't document. Advanced users can use _run_command to get something going but for larger applications, they should request a API to be added here.

PMeira commented 4 years ago

@julioolvera1 @kdheepak Solution.LoadMult() already exists (and does work, I use it myself -- the load multiplier is not always used in OpenDSS, there are exceptions). I know there are other missing properties though. If you can think of any important ones, please report so we can properly expose them. I should merge some local changes to on DSS C-API in the next couple of weeks, I can take some time to add the Pascal code needed for the extra API properties too.

In my own code and most of my team's, the text interface is mostly used for some set ... and BatchEdit ... calls, besides redirects/compiles. Some classes are missing completely from the API though, and we're aware of that. I'll need more time to finish some work on the OpenDSS property system (large changeset, affects all OpenDSS components) but it should be done in some months.

The text interface for commands is still required since the DSS language is a big part of OpenDSS, at least for the moment. Many of the commands are actually available right now -- there are some extensions we didn't port from dss_python to ODD.py, but it's not that much. When we finish exposing all properties, we should really push for more pythonic code.

Perhaps the solution is to deprecate run_command but expose a _run_command that we don't document. Advanced users can use _run_command to get something going but for larger applications, they should request a API to be added here.

I think running CheckForError after each line to throw an exception is the preferred solution, I also learned that people really don't check for errors manually. Most people don't even check if the power flow solution converged!

At the same time, I don't think most people need the results for each of the commands. I was planning on adding a dss_lib.Text_Set_Commands in Pascal to run a bunch of lines like a .DSS script file, as the current dss_lib.Text_Set_Command wouldn't work for that (i.e., without ...splitlines()).
@kdheepak, do you think adding an option in Pascal to return the results of this new dss_lib.Text_Set_Commands as a string would be useful, or just the error message returned through the exception (from Error_Get_Description) is enough?

Note that an error while processing this multi-line string in Pascal would halt the execution early when DSS_CAPI_EARLY_ABORT is on (default) -- we need to expose this in ODD.py too per https://github.com/dss-extensions/OpenDSSDirect.py/issues/69#issuecomment-501777700

From time to time people report some issues (bugs or corner cases) in OpenDSS that have been present for many years. This reminded of this post -- indeed it seems most people ignore errors, unexpected behavior and missing features: https://pointersgonewild.com/2019/11/02/they-might-never-tell-you-its-broken/