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

Feature: Add SavingOptions to save_result API #874

Closed

Conversation

joernweissenborn
Copy link
Member

This PR adds the SavingOption to the save_result function. It also adds more tests for the result.

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)
  • 📚 Adds documentation of the feature

Closes issues

closes #XXXX

@joernweissenborn joernweissenborn requested review from jsnel, s-weigand and a team as code owners October 19, 2021 18:35
@github-actions
Copy link
Contributor

Binder 👈 Launch a binder notebook on branch joernweissenborn/pyglotaran/refactor/save_result

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
5.9% 5.9% Duplication

@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2021

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

Benchmark diff v0.5.1 vs. main

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)

All benchmarks:

       before           after         ratio
     [96b42630]       [fdb35f91]
     <v0.5.1>                   
       60.4±0.6ms       59.4±0.4ms     0.98  BenchmarkOptimize.time_optimize(False, False, False)
         64.1±1ms         65.5±3ms     1.02  BenchmarkOptimize.time_optimize(False, False, True)
       59.6±0.6ms       59.4±0.4ms     1.00  BenchmarkOptimize.time_optimize(False, True, False)
        67.2±30ms        79.7±30ms    ~1.19  BenchmarkOptimize.time_optimize(False, True, True)
       71.5±0.9ms         71.7±1ms     1.00  BenchmarkOptimize.time_optimize(True, False, False)
        77.3±30ms        80.3±20ms     1.04  BenchmarkOptimize.time_optimize(True, False, True)
         71.4±1ms         71.5±1ms     1.00  BenchmarkOptimize.time_optimize(True, True, False)
         109±40ms         76.5±4ms    ~0.70  BenchmarkOptimize.time_optimize(True, True, True)
             202M             201M     1.00  IntegrationTwoDatasets.peakmem_optimize
       2.00±0.04s       1.92±0.04s     0.96  IntegrationTwoDatasets.time_optimize

Benchmark diff main vs. PR

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)

All benchmarks:

       before           after         ratio
     [09de6b26]       [fdb35f91]
       59.6±0.6ms       59.4±0.4ms     1.00  BenchmarkOptimize.time_optimize(False, False, False)
        78.4±30ms         65.5±3ms    ~0.84  BenchmarkOptimize.time_optimize(False, False, True)
       58.6±0.4ms       59.4±0.4ms     1.01  BenchmarkOptimize.time_optimize(False, True, False)
        64.0±20ms        79.7±30ms    ~1.24  BenchmarkOptimize.time_optimize(False, True, True)
       71.4±0.6ms         71.7±1ms     1.00  BenchmarkOptimize.time_optimize(True, False, False)
        82.7±30ms        80.3±20ms     0.97  BenchmarkOptimize.time_optimize(True, False, True)
       71.8±0.4ms         71.5±1ms     1.00  BenchmarkOptimize.time_optimize(True, True, False)
         111±30ms         76.5±4ms    ~0.69  BenchmarkOptimize.time_optimize(True, True, True)
             204M             201M     0.99  IntegrationTwoDatasets.peakmem_optimize
       1.94±0.03s       1.92±0.04s     0.99  IntegrationTwoDatasets.time_optimize

@codecov
Copy link

codecov bot commented Dec 2, 2021

Codecov Report

Merging #874 (fdb35f9) into main (09de6b2) will increase coverage by 0.1%.
The diff coverage is 96.4%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #874     +/-   ##
=======================================
+ Coverage   85.3%   85.5%   +0.1%     
=======================================
  Files         90      90             
  Lines       4857    4870     +13     
  Branches     912     913      +1     
=======================================
+ Hits        4147    4166     +19     
+ Misses       561     553      -8     
- Partials     149     151      +2     
Impacted Files Coverage Δ
.../builtin/io/ascii/wavelength_time_explicit_file.py 58.6% <ø> (ø)
glotaran/plugin_system/data_io_registration.py 97.2% <ø> (ø)
glotaran/builtin/io/folder/folder_plugin.py 95.8% <93.9%> (-1.8%) ⬇️
glotaran/builtin/io/yml/yml.py 89.0% <100.0%> (+0.4%) ⬆️
glotaran/io/__init__.py 100.0% <100.0%> (ø)
glotaran/io/interface.py 100.0% <100.0%> (ø)
glotaran/plugin_system/project_io_registration.py 100.0% <100.0%> (ø)
glotaran/project/result.py 88.4% <100.0%> (+7.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 09de6b2...fdb35f9. Read the comment docs.

@joernweissenborn joernweissenborn force-pushed the refactor/save_result branch 2 times, most recently from 966c524 to 72e250c Compare December 3, 2021 12:58
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 3, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
17.5% 17.5% Duplication

@jsnel jsnel force-pushed the refactor/save_result branch from 72e250c to 671c6bd Compare January 16, 2022 11:50
@s-weigand s-weigand added the Type: Enhancement Feature requests label Jan 16, 2022
@joernweissenborn joernweissenborn changed the title Refactor/save result Feature: Add SavingOptions to sve_result API Jan 22, 2022
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 22, 2022

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 0.52%.

Quality metrics Before After Change
Complexity 1.56 ⭐ 1.61 ⭐ 0.05 👎
Method Length 36.66 ⭐ 37.50 ⭐ 0.84 👎
Working memory 5.80 ⭐ 5.91 ⭐ 0.11 👎
Quality 83.50% 82.98% -0.52% 👎
Other metrics Before After Change
Lines 2646 2842 196
Changed files Quality Before Quality After Quality Change
glotaran/builtin/io/ascii/wavelength_time_explicit_file.py 78.10% ⭐ 78.01% ⭐ -0.09% 👎
glotaran/builtin/io/folder/folder_plugin.py 59.69% 🙂 62.16% 🙂 2.47% 👍
glotaran/builtin/io/folder/test/test_folder_plugin.py 87.00% ⭐ 86.36% ⭐ -0.64% 👎
glotaran/builtin/io/yml/yml.py 77.86% ⭐ 77.36% ⭐ -0.50% 👎
glotaran/builtin/io/yml/test/test_save_result.py 87.74% ⭐ 81.46% ⭐ -6.28% 👎
glotaran/io/init.py 87.99% ⭐ 87.99% ⭐ 0.00%
glotaran/io/interface.py 97.71% ⭐ 92.06% ⭐ -5.65% 👎
glotaran/plugin_system/data_io_registration.py 84.42% ⭐ 84.36% ⭐ -0.06% 👎
glotaran/plugin_system/project_io_registration.py 83.86% ⭐ 84.70% ⭐ 0.84% 👍
glotaran/plugin_system/test/test_data_io_registration.py 90.24% ⭐ 90.13% ⭐ -0.11% 👎
glotaran/plugin_system/test/test_project_io_registration.py 90.24% ⭐ 90.16% ⭐ -0.08% 👎
glotaran/project/result.py 77.52% ⭐ 77.38% ⭐ -0.14% 👎
glotaran/project/test/test_result.py 93.62% ⭐ 82.73% ⭐ -10.89% 👎

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
glotaran/builtin/io/yml/yml.py YmlProjectIo.save_model 15 🙂 106 🙂 13 😞 48.93% 😞 Extract out complex expressions
glotaran/project/result.py Result.markdown 6 ⭐ 173 😞 13 😞 49.52% 😞 Try splitting into smaller methods. Extract out complex expressions
glotaran/builtin/io/ascii/wavelength_time_explicit_file.py ExplicitFile.write 5 ⭐ 170 😞 13 😞 50.77% 🙂 Try splitting into smaller methods. Extract out complex expressions
glotaran/builtin/io/ascii/wavelength_time_explicit_file.py ExplicitFile.read 4 ⭐ 138 😞 12 😞 56.63% 🙂 Try splitting into smaller methods. Extract out complex expressions
glotaran/builtin/io/folder/folder_plugin.py save_result_to_folder 3 ⭐ 218 ⛔ 8 🙂 58.41% 🙂 Try splitting into smaller methods

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

@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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@joernweissenborn joernweissenborn changed the title Feature: Add SavingOptions to sve_result API Feature: Add SavingOptions to save_result API Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants