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

SRR and CODS updates #426

Closed
wants to merge 32 commits into from
Closed

SRR and CODS updates #426

wants to merge 32 commits into from

Conversation

martin-springer
Copy link
Collaborator

  • 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

nmoyer and others added 6 commits June 24, 2024 11:57
@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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
@@                        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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@martin-springer martin-springer changed the base branch from development to aggregated_filters_for_trials August 6, 2024 17:41
@martin-springer
Copy link
Collaborator Author

@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
Copy link
Collaborator

@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
Copy link
Collaborator Author

@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
Copy link
Collaborator

@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

Base automatically changed from aggregated_filters_for_trials to development October 29, 2024 03:30
@mdeceglie
Copy link
Collaborator

Closing in favor of #432

@mdeceglie mdeceglie closed this Nov 4, 2024
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.

4 participants