Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up iterator interface #55

Closed
kdheepak opened this issue Feb 17, 2019 · 9 comments · Fixed by #78
Closed

Clean up iterator interface #55

kdheepak opened this issue Feb 17, 2019 · 9 comments · Fixed by #78
Assignees

Comments

@kdheepak
Copy link
Member

Follow up to discussion here.

@PMeira can you explain what your thoughts are on this?

@PMeira
Copy link
Member

PMeira commented Feb 17, 2019

are suggesting changing all the modules to classes in ODD.py? We should open an issue over there to discuss this further.

@kdheepak Modules have several restrictions; Python 3.8 (to be released in Oct/2019) will allow some more freedom but it's a bit late.

With classes we can use a lot of "dunder" methods to make things more natural, and even provide support for both lines of OpenDSS (v7 and v8, although there isn't much demand for v8 at the moment). If we ever get an alternative library with the same API, it would be easier to reuse the current code too, without duplicating the code.

Each class gets the api_util class and instantiates the children classes. So IDSS instantiates everything:

class IDSS(Base):
#...
    def __init__(self, api_util):
        self.ActiveCircuit = ICircuit(api_util)
        self.Error = IError(api_util)
        self.Text = IText(api_util)
#...

Of course, OpenDSS right now is a singleton, so if you create another instance, it's the same thing; it makes sense to expose dss.DSS as an already initialized instance.

In dss_python there is support for simple iterators for most classes and indexing for some special classes. Part of it I added to mimic COM access, part I added as extra since it would not hurt.

from dss import DSS

DSS.Text.Command = r'compile "IEEETestCases/13Bus/IEEE13Nodeckt.dss"'

element = DSS.ActiveCircuit.ActiveElement    
buses = DSS.ActiveCircuit.Buses
lines = DSS.ActiveCircuit.Lines

# These implement __iter__
for bus in buses:
    print(bus.Name, bus.puVoltages)

for line in lines:
    print(line.Name, line.Bus1, line.Bus2)

# __len__ is available too
print("Number of lines:", len(lines)) 

# Classes like "Properties" and "Buses" implement __getitem__ 
# (and __call__ for win32com compatibility)
print(
    element.Name,
    element.Properties['Bus1'].Val, 
    element.Properties['Bus2'].Val
)


bus = buses(10)
print(bus.Name, bus.puVoltages)

bus = buses[8]
print(bus.Name, bus.puVoltages)

We can even monkey-patch the COM classes to support it.

I haven't done it yet, but we could add __str__ and __repr__ methods for pretty-printing and printing the DSS script code for the element, respectively.

Using classes we can also use mixins (and metaclasses) to reduce repeated code patterns in some situations. For example, right now only a few classes has the New function, but for 0.11 will allow New on every class. I tested using a HasNew mixin to change from this:

class ILines(Base):
    __slots__ = []

    def New(self, Name):
        if type(Name) is not bytes:
            Name = Name.encode(self._api_util.codec)

        return self._lib.Lines_New(Name)
#...

to:

class ILines(Base, HasDSSNew):
    __slots__ = []

#...

Later could even add HasDSSIteration to removed the repeated code for First/Next/Count/AllNames/etc.

class ILines(Base, HasNew, HasDSSIteration):
    __slots__ = []

#...

There is some more magic in the background, but it makes it easier to add new features.

I was also thinking in using metaclasses to call CheckForError without polluting the code too much.

The __slots__ usage makes things a bit faster in CPython, and avoids the issue of creating new variables in the class/module by accident, common with new users (this is probably more pronounced in the COM-like API due to overuse of properties, shouldn't affect ODD.py).

Long term we should change the iteration pattern in (our version of) OpenDSS as a whole, but that's more intrusive and not worth yet.

@kdheepak
Copy link
Member Author

kdheepak commented Feb 17, 2019

I like everything you've suggested.

Regarding the module interface, my only concern is maintaining backward compatibility. If someone has written code like so:

import opendssdirect as dss
dss.run_command("Redirect tests/data/13Bus/Master.dss")
...
dss.Lines.Name()

Then it won't matter to them if Lines is a Python module or a Class. However, if they have done this:

import opendssdirect as dss
dss.run_command("Redirect tests/data/13Bus/Master.dss")
...
from opendssdirect.Lines import Name
Name()

Then this will be a breaking change, right?

I'm still for moving to a class based interface though, it'll simplify the code and make it easier to write nicer Pythonic interfaces for us as developers and for users.

My suggestion is that we introduce a new major version where we add warnings and indicate that we are going to deprecate the second interface, i.e. modules. It can be during import or during run time. And in the same major version have the classes interface as well. Then the next major version release can remove the module interface.

Thoughts?

@PMeira
Copy link
Member

PMeira commented Feb 17, 2019

Then this will be a breaking change, right?

ODD.py doesn't use properties so we could keep the modules as a compatibility layer, but probably warn anyway:

# Lines.py
import opendssdirect as dss

Name = dss.Lines.Name
First = dss.Lines.First
Next = dss.Lines.Next

__all__ = [...]

My suggestion is that we introduce a new major version where we add warnings and indicate that we are going to deprecate the second interface, i.e. modules. It can be during import or during run time. And in the same major version have the classes interface as well. Then the next major version release can remove the module interface.

I'm fine with that, after DSS C-API 0.11 we probably will add a lot of new methods, so it could go along with it.

@kdheepak
Copy link
Member Author

This will work great!

@kdheepak kdheepak self-assigned this Mar 5, 2019
@kdheepak
Copy link
Member Author

kdheepak commented Mar 5, 2019

I'll likely be able to tackle this in the next month or so.

@PMeira
Copy link
Member

PMeira commented Mar 7, 2019

@kdheepak I'm starting some related work on dss_python that can probably be shared to simplify things. We should start a branch here soon.

@PMeira
Copy link
Member

PMeira commented Mar 7, 2019

@kdheepak Update: take a look at dss-extensions/DSS-Python@92bf868

It removed around 2k lines of code, even though the commit also added idx access for a lot of classes.

Since the group of functions were implemented in C-API, only one extra class was needed here. For more complicated things, mixins should work in some scenarios. I also tested some metaclasses but, since it's one of the things that change between Python 2 and 3, I didn't use them.

@kdheepak
Copy link
Member Author

kdheepak commented Mar 7, 2019

Interesting! That looks really neat! When documentation is generated can we make the child classes' documentation page show all the methods?

@PMeira
Copy link
Member

PMeira commented Mar 7, 2019

Yes, in Sphinx adding :inherited-members: does that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants