SRR and CODS updates - Githubissues

NREL / rdtools

PV Analysis Tools in Python

https://rdtools.readthedocs.io/

MIT License

158 stars 67 forks source link

SRR and CODS updates #426

Closed martin-springer closed 3 weeks ago

martin-springer commented 4 months ago

[ ] Code changes are covered by tests
[ ] Code changes have been evaluated for compatibility/integration with TrendAnalysis
[ ] New functions added to __init__.py
[ ] API.rst is up to date, along with other sphinx docs pages
[ ] Example notebooks are rerun and differences in results scrutinized
[ ] Updated changelog

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.68%. Comparing base (e4349b1) to head (f64077a). Report is 25 commits behind head on aggregated_filters_for_trials.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## aggregated_filters_for_trials #426 +/- ## ================================================================= - Coverage 96.10% 95.68% -0.42% ================================================================= Files 11 11 Lines 2206 2366 +160 ================================================================= + Hits 2120 2264 +144 - Misses 86 102 +16 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

martin-springer commented 3 months ago

@noromo01 - I tried to help with the soiling.py formatting. Not sure, if I made it better or worse tough... However, all of those can be fixed easily. I can take care of that, if you like? The only one that I need your input is the last exception in soiling.py (./rdtools/soiling.py:2944:5: E722 do not use bare 'except'). What exception would you like to catch there?

noromo01 commented 3 months ago

@martin-springer I believe I fixed the formatting issues, everything looks good to me except for that bare except. I talked to Matt about that yesterday, we are working to find a solution to that. Someone else wrote that code so we are having some difficulty figuring out how to fix that

martin-springer commented 3 months ago

@martin-springer I believe I fixed the formatting issues, everything looks good to me except for that bare except. I talked to Matt about that yesterday, we are working to find a solution to that. Someone else wrote that code so we are having some difficulty figuring out how to fix that

@noromo01 - Got it. Thanks for fixing the formatting. It's looking good now. Please add your changes to the changelog under (docs\sphinx\source\changelog\pending.rst).

@mdeceglie - Besides the one bare exception in soiling.py, this PR is ready for your review.

noromo01 commented 3 months ago

@martin-springer I believe I fixed the formatting issues, everything looks good to me except for that bare except. I talked to Matt about that yesterday, we are working to find a solution to that. Someone else wrote that code so we are having some difficulty figuring out how to fix that

@noromo01 - Got it. Thanks for fixing the formatting. It's looking good now. Please add your changes to the changelog under (docs\sphinx\source\changelog\pending.rst).

@mdeceglie - Besides the one bare exception in soiling.py, this PR is ready for your review.

@martin-springer @mdeceglie Matt and I are still working out some of the kinks with the notebook testing. We got everything to look good for method='half_norm_clean' but are still trying to make sure we don't change the functionality of Matt's new methods with piecewise and negative shift detection

mdeceglie commented 3 weeks ago

Closing in favor of #432