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

DSS_Python 0.10.5, API sync and better error handling #77

Merged
merged 6 commits into from
Jun 17, 2020

Conversation

PMeira
Copy link
Member

@PMeira PMeira commented Mar 2, 2020

Upgrade do DSS_Python 0.10.5, synchronizing the API (still maintain ODD.py's conventions):

  • Add previously missing modules (API extensions)
  • Add CheckForError for most setters
  • Update many docstrings
  • Introduce many new functions/properties, either new functions from DSS C-API or functions ported from the official COM module API
  • Remove some functions with side-effects from _columns

Closes #42, closes #83, potentially #69 as well (needs test).

Remaining tasks:

  • Update tests after the API sync

@PMeira PMeira changed the title Dss python 0.10.5 DSS_Python 0.10.5 Mar 2, 2020
@PMeira
Copy link
Member Author

PMeira commented Mar 2, 2020

Tests passing in dss_python, odd.jl and here in odd.py. I'll try to release tomorrow to allow us to merge this.

Like I mentioned in #75 and dss-extensions/dss_capi#74, the new components were added to the base engine but they're kinda buggy still (even on the official OpenDSS).

@PMeira PMeira marked this pull request as ready for review March 9, 2020 04:39
@PMeira
Copy link
Member Author

PMeira commented Mar 9, 2020

@kdheepak I think this is good to go if the CI tests pass OK.

I started adding missing functions, which are mostly many DSS C-API extensions and a couple of new functions ported from the COM API, but I decided to skip those in this PR and approach them more automatically.

I started writing a basic transformation to port the useful code from dss_python to oddpy, akin to what I did in the initial dss_capi migration but directly on the Python code this time. I'm planning on doing this on 2 steps:

This will help keep the implementations in sync (in terms of new dss_capi functions) and reduce (manual) coding errors.

@PMeira PMeira requested a review from kdheepak March 9, 2020 04:56
@PMeira PMeira changed the title DSS_Python 0.10.5 DSS_Python 0.10.5, API sync and better error handling Mar 11, 2020
@PMeira
Copy link
Member Author

PMeira commented Mar 11, 2020

I started writing a basic transformation to port the useful code from dss_python to oddpy, akin to what I did in the initial dss_capi migration but directly on the Python code this time. I'm planning on doing this on 2 steps:

I was able to finish this, but still need to update the tests again after the changes. Some are failing due to previous misuse, some just because I changed the available _columns for the dataframes.

@kdheepak
Copy link
Member

This is great!

odd.Circuit.Variable(MyVarName, Code) -> odd.Circuit.Variable(MyVarName)
odd.Circuit.Variablei(Idx, Code) -> odd.Circuit.Variablei(Idx)

Technically this is breaking. I think these are the only two functions though. We could add a Code=None in the function definition and add a deprecation warning that triggers if Code is not None in the function body. And in the next version we can make the change as in this PR.

Also, this PR will now cause functions to error (as it should) where as previously if something went wrong it would be silent and return. I would not expect that from v0.3.7 to v0.3.8 of a package to cause this, even if it the correct thing to do and even if I would prefer it. Thoughts?

Maybe we can just update to v0.4.0 before tagging?

@PMeira
Copy link
Member Author

PMeira commented Mar 11, 2020

odd.Circuit.Variable(MyVarName, Code) -> odd.Circuit.Variable(MyVarName)
odd.Circuit.Variablei(Idx, Code) -> odd.Circuit.Variablei(Idx)

Technically this is breaking. I think these are the only two functions though. We could add a Code=None in the function definition and add a deprecation warning that triggers if Code is not None in the function body. And in the next version we can make the change as in this PR.

That's a good idea. I can enable the test for that too.

Maybe we can just update to v0.4.0 before tagging?

Yes, incrementing just the micro version could be misleading. Since there are a bunch of new functions and modules, I think jumping to v0.4.0 is fine.

After #55, we may need to do it again, depending on how it ends up looking.

I will try to work on updating the tests tonight. Soon after that, I'll start another branch/PR about the class-based version and iterators. It should be a relatively quick change but certainly will require at least some new tests to ensure it's working as expected and mostly compatible with the current version.

We probably can wait a few more days and then announce the resulting version on the official forums to reach more users. I noticed more and more users of ODD.py posting there.

Copy link
Member Author

@PMeira PMeira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kdheepak I think this addresses the main backwards compatibility issue. Since we're not limited to the COM API compatibility like in dss_python, I mapped the Code value to an exception. I added your suggestion of keeping the input Code param as well as the warning.

The new tests for this are included in the commit that follows this one.

@codecov-io
Copy link

codecov-io commented Mar 12, 2020

Codecov Report

Merging #77 into master will decrease coverage by 12.89%.
The diff coverage is 26.43%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #77       +/-   ##
===========================================
- Coverage   68.65%   55.76%   -12.90%     
===========================================
  Files          47       54        +7     
  Lines        3053     4261     +1208     
===========================================
+ Hits         2096     2376      +280     
- Misses        957     1885      +928     
Impacted Files Coverage Δ
opendssdirect/__init__.py 100.00% <ø> (ø)
opendssdirect/LineCodes.py 22.42% <5.26%> (-4.24%) ⬇️
opendssdirect/Loads.py 50.20% <7.31%> (-8.10%) ⬇️
opendssdirect/Generators.py 59.09% <7.69%> (-8.00%) ⬇️
opendssdirect/Reclosers.py 57.44% <7.69%> (-7.26%) ⬇️
opendssdirect/Settings.py 47.10% <8.69%> (-9.46%) ⬇️
opendssdirect/CapControls.py 50.00% <9.52%> (-9.38%) ⬇️
opendssdirect/Fuses.py 58.75% <10.00%> (-6.12%) ⬇️
opendssdirect/Parser.py 27.27% <14.28%> (-3.88%) ⬇️
opendssdirect/RegControls.py 52.56% <14.28%> (-8.51%) ⬇️
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e64f25...f4725a1. Read the comment docs.

@PMeira
Copy link
Member Author

PMeira commented Mar 12, 2020

The latest push fixed the CI configs to use the main PyPI instead of the test instance (test.pypi.org) when installing dss_python. I'll upload the pre-releases to the main instance next time to avoid this kind of change.

- Update tests to DSS C-API/Python 0.10.5
- setup.py: Update the dependency on dss_python
- setup.py: Add long_description_content_type to render the README correctly on PyPI
- On CI, allow pip to install pre-releases for easier testing of pre-releases
- Add CheckForError for most setters
- Update many docstrings
- Introduce many new functions/properties
- Remove some functions with side-effects from `_columns`
- Add tests CktElement.Variable/Variablei
- Reenable/fix some older commented tests
- Complement the DataFrames with the new fields
@PMeira
Copy link
Member Author

PMeira commented Apr 23, 2020

@kdheepak Do you mind if I merge this one and prepare a release on the weekend?

@kdheepak
Copy link
Member

I think it looks good! Sorry for the delayed responses.

@PMeira PMeira merged commit 1e518d2 into master Jun 17, 2020
@PMeira PMeira deleted the dss_python-0.10.5 branch June 17, 2020 14:54
PMeira added a commit that referenced this pull request Jun 17, 2020
Minor version increment as discussed in #77 (comment)
i.e. new error-handling introduced, using frequent checks to map the errors to exceptions
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 this pull request may close these issues.

update dss_python version from requirements Daily is not present in PVSystems.
3 participants