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

Update calls to clp_area_penalties where deprecated func is called #790

Merged
merged 1 commit into from
Aug 25, 2021

Conversation

jsnel
Copy link
Member

@jsnel jsnel commented Aug 23, 2021

The deprecated equal_area_penalties would be called in lieu of the new clp_area_penalties function

The pyglotaran-examples have in their model still equal_area_penalties which is correctly swapped with clp_area_penalties but them model.equal_area_penalties was still called in some places instead of model.clp_area_penalties

Checklist

  • ✔️ Passing the tests (mandatory for all PR's)
  • 👌 Closes issue (mandatory for ✨ feature and 🩹 bug fix PR's)
  • 🧪 Adds new tests for the feature (mandatory for ✨ feature and 🩹 bug fix PR's)

Closes issues

closes #789

The deprecated equal_area_penalties would be called in lieu of the new clp_area_penalties function

The pyglotaran-examples have in their model still equal_area_penalties which is correctly swapped with clp_area_penalties but them model.equal_area_penalties  was still called in some places instead of model.clp_area_penalties
@github-actions
Copy link
Contributor

Binder 👈 Launch a binder notebook on branch jsnel/pyglotaran/fix/789_calls_to_clp_area_penalties

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Aug 23, 2021

Codecov Report

Merging #790 (95a8377) into staging (5e9eb22) will not change coverage.
The diff coverage is 0.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           staging    #790   +/-   ##
=======================================
  Coverage     83.9%   83.9%           
=======================================
  Files           75      75           
  Lines         4185    4185           
  Branches       752     752           
=======================================
  Hits          3512    3512           
  Misses         538     538           
  Partials       135     135           
Impacted Files Coverage Δ
glotaran/model/clp_penalties.py 31.9% <0.0%> (ø)

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 5e9eb22...95a8377. Read the comment docs.

@github-actions
Copy link
Contributor

Benchmark is done. Checkout the benchmark result page.
Benchmark differences below 5% might be due to CI noise.

Benchmark diff

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)

All benchmarks:

       before           after         ratio
     [dc00e6da]       [95a8377d]
     <v0.4.0>                   
-        67.4±1ms         45.7±1ms     0.68  BenchmarkOptimize.time_optimize(False, False, False)
-         377±3ms        57.9±10ms     0.15  BenchmarkOptimize.time_optimize(False, False, True)
-        92.7±2ms         75.8±1ms     0.82  BenchmarkOptimize.time_optimize(False, True, False)
         94.9±1ms         86.5±4ms     0.91  BenchmarkOptimize.time_optimize(False, True, True)
         65.2±2ms         62.4±1ms     0.96  BenchmarkOptimize.time_optimize(True, False, False)
-         374±4ms        69.5±10ms     0.19  BenchmarkOptimize.time_optimize(True, False, True)
         93.3±1ms         94.8±2ms     1.02  BenchmarkOptimize.time_optimize(True, True, False)
         96.2±1ms         105±50ms     1.09  BenchmarkOptimize.time_optimize(True, True, True)
             180M             179M     0.99  IntegrationTwoDatasets.peakmem_create_result
             197M             197M     1.00  IntegrationTwoDatasets.peakmem_optimize
-         288±3ms          236±3ms     0.82  IntegrationTwoDatasets.time_create_result
-      5.95±0.02s       2.00±0.07s     0.34  IntegrationTwoDatasets.time_optimize

@jsnel
Copy link
Member Author

jsnel commented Aug 23, 2021

Ignore the codecov result comment which fails because the commit change single line function which are not under coverage so the relative change is through the roof even though the code coverage in absolute sense doesn't change.

Copy link
Member

@joernweissenborn joernweissenborn left a comment

Choose a reason for hiding this comment

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

Typical joern copy paste bugs, thx for fixing

@jsnel jsnel merged commit dd0ce80 into glotaran:staging Aug 25, 2021
@jsnel jsnel deleted the fix/789_calls_to_clp_area_penalties branch August 25, 2021 01:36
jsnel added a commit that referenced this pull request Sep 16, 2021
…790)

The deprecated equal_area_penalties would be called in lieu of the new clp_area_penalties function

The pyglotaran-examples have in their model still equal_area_penalties which is correctly swapped with clp_area_penalties but them model.equal_area_penalties  was still called in some places instead of model.clp_area_penalties
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.

2 participants