✨Make guidance spectra a first class citizen #983

s-weigand · 2022-01-30T20:04:51Z

This is just a preview for maintainers

Change summary

✨ Initial draft for a simplified specification of guidance spectra
♻️🩹 Renamed overwrite_{model|global}dimension to force{model|global}_dimension
🩹 Fixed tests
♻️ Renamed attributes force_{model|global}dimension back to overwrite{model|global}_dimension
♻️ Renamed methods overwrite_{model|global}dimension to replace{model|global}_dimension

Checklist

✔️ Passing the tests (mandatory for all PR's)
🚧 Added changes to changelog (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 #846

github-actions · 2022-01-30T20:05:03Z

👈 Launch a binder notebook on branch s-weigand/pyglotaran/simplify-guidance

sonarqubecloud · 2022-01-30T20:05:27Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

joernweissenborn

lgtrm

github-actions · 2022-01-30T20:11:38Z

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]       [a4df8f93]
     <v0.5.1>                   
       49.2±0.1ms       49.4±0.4ms     1.00  BenchmarkOptimize.time_optimize(False, False, False)
       53.0±0.5ms       53.1±0.7ms     1.00  BenchmarkOptimize.time_optimize(False, False, True)
       49.6±0.6ms       49.7±0.4ms     1.00  BenchmarkOptimize.time_optimize(False, True, False)
        54.7±20ms        54.3±10ms     0.99  BenchmarkOptimize.time_optimize(False, True, True)
         59.6±2ms       60.0±0.5ms     1.01  BenchmarkOptimize.time_optimize(True, False, False)
        68.8±40ms        63.6±30ms     0.92  BenchmarkOptimize.time_optimize(True, False, True)
       59.7±0.2ms       59.7±0.4ms     1.00  BenchmarkOptimize.time_optimize(True, True, False)
        64.1±50ms        65.6±30ms     1.02  BenchmarkOptimize.time_optimize(True, True, True)
             201M             206M     1.02  IntegrationTwoDatasets.peakmem_optimize
       1.71±0.08s       1.61±0.04s     0.94  IntegrationTwoDatasets.time_optimize

Benchmark diff main vs. PR

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)


All benchmarks:

       before           after         ratio
     [d15d9bd3]       [a4df8f93]
       49.8±0.3ms       49.4±0.4ms     0.99  BenchmarkOptimize.time_optimize(False, False, False)
       53.1±0.4ms       53.1±0.7ms     1.00  BenchmarkOptimize.time_optimize(False, False, True)
       49.3±0.6ms       49.7±0.4ms     1.01  BenchmarkOptimize.time_optimize(False, True, False)
         57.5±5ms        54.3±10ms     0.94  BenchmarkOptimize.time_optimize(False, True, True)
         66.9±7ms       60.0±0.5ms    ~0.90  BenchmarkOptimize.time_optimize(True, False, False)
        74.3±40ms        63.6±30ms    ~0.86  BenchmarkOptimize.time_optimize(True, False, True)
        65.2±10ms       59.7±0.4ms     0.92  BenchmarkOptimize.time_optimize(True, True, False)
        84.3±50ms        65.6±30ms    ~0.78  BenchmarkOptimize.time_optimize(True, True, True)
             202M             206M     1.02  IntegrationTwoDatasets.peakmem_optimize
       1.64±0.05s       1.61±0.04s     0.98  IntegrationTwoDatasets.time_optimize

codecov · 2022-01-30T20:12:58Z

Codecov Report

Merging #983 (a4df8f9) into main (279b24c) will decrease coverage by 0.2%.
The diff coverage is 58.3%.

@@           Coverage Diff           @@
##            main    #983     +/-   ##
=======================================
- Coverage   85.9%   85.7%   -0.3%     
=======================================
  Files         92      93      +1     
  Lines       4972    5002     +30     
  Branches     937     942      +5     
=======================================
+ Hits        4272    4287     +15     
- Misses       552     564     +12     
- Partials     148     151      +3

Impacted Files	Coverage Δ
glotaran/model/model.py	`85.5% <0.0%> (-0.1%)`	⬇️
glotaran/model/dataset_model.py	`76.9% <58.6%> (-5.1%)`	⬇️
glotaran/model/clp_guidance_megacomplex.py	`61.1% <61.1%> (ø)`

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 279b24c...a4df8f9. Read the comment docs.

glotaran/model/dataset_model.pyi

glotaran/model/dataset_model.py

glotaran/model/model.py

jsnel

Naming things ...

glotaran/model/dataset_model.pyi

Co-authored-by: Jörn Weißenborn <[email protected]> Co-authored-by: Joris Snellenburg <[email protected]>

…}_dimension So they won't conflict with the method which has the same name

overwrite_{model|global}_dimension ♻️ Renamed methods overwrite_{model|global}_dimension to replace_{model|global}_dimension

to set_{model|global}_dimension

glotaran/model/model.py

s-weigand · 2022-03-25T19:03:39Z

As discussed with @joernweissenborn today he will implement this as a builtin megacomplex because it will make it more consistent with the modeling language spec and the theory description in "the book" (his thesis).

@joernweissenborn please also note this discussion

sourcery-ai · 2022-03-26T12:16:28Z

Sourcery Code Quality Report

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

Quality metrics	Before	After	Change
Complexity	5.38 ⭐	5.52 ⭐	0.14 👎
Method Length	53.32 ⭐	53.16 ⭐	-0.16 👍
Working memory	7.88 🙂	7.91 🙂	0.03 👎
Quality	68.77% 🙂	68.59% 🙂	-0.18% 👎

Other metrics	Before	After	Change
Lines	876	897	21

Changed files	Quality Before	Quality After	Quality Change
glotaran/builtin/megacomplexes/baseline/test/test_baseline_megacomplex.py	65.48% 🙂	65.48% 🙂	0.00%
glotaran/builtin/megacomplexes/coherent_artifact/test/test_coherent_artifact.py	42.64% 😞	42.64% 😞	0.00%
glotaran/builtin/megacomplexes/spectral/test/test_spectral_model.py	61.32% 🙂	61.32% 🙂	0.00%
glotaran/model/dataset_model.py	80.93% ⭐	79.35% ⭐	-1.58% 👎
glotaran/model/model.py	70.83% 🙂	70.55% 🙂	-0.28% 👎

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

File	Function	Complexity	Length	Working Memory	Quality	Recommendation
glotaran/builtin/megacomplexes/coherent_artifact/test/test_coherent_artifact.py	test_coherent_artifact	6 ⭐	459 ⛔	11 😞	42.64% 😞	Try splitting into smaller methods. Extract out complex expressions
glotaran/model/model.py	Model.markdown	13 🙂	168 😞	10 😞	48.74% 😞	Try splitting into smaller methods. Extract out complex expressions
glotaran/model/model.py	Model.from_dict	12 🙂	162 😞	10 😞	50.19% 🙂	Try splitting into smaller methods. Extract out complex expressions
glotaran/model/dataset_model.py	DatasetModel.get_global_dimension	15 🙂	128 😞	10 😞	51.34% 🙂	Try splitting into smaller methods. Extract out complex expressions
glotaran/builtin/megacomplexes/spectral/test/test_spectral_model.py	test_spectral_model	1 ⭐	237 ⛔	9 🙂	57.03% 🙂	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 · 2022-03-26T12:16:53Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

s-weigand · 2022-03-26T16:51:59Z

Superseded by #1029

sourcery-ai bot mentioned this pull request Jan 30, 2022

✨Make guidance spectra a first class citizen (Sourcery refactored) #984

Closed

joernweissenborn previously approved these changes Jan 30, 2022

View reviewed changes

jsnel changed the title ~~✨Make guidance spectra a first class thing~~ ✨Make guidance spectra a first class citizen Jan 30, 2022

jsnel reviewed Feb 14, 2022

View reviewed changes

glotaran/model/dataset_model.pyi Outdated Show resolved Hide resolved

jsnel reviewed Feb 14, 2022

View reviewed changes

glotaran/model/dataset_model.py Outdated Show resolved Hide resolved

jsnel reviewed Feb 14, 2022

View reviewed changes

glotaran/model/model.py Outdated Show resolved Hide resolved

jsnel requested changes Feb 14, 2022

View reviewed changes

s-weigand dismissed joernweissenborn’s stale review via d0d354f March 10, 2022 17:00

s-weigand force-pushed the simplify-guidance branch from 645da52 to d0d354f Compare March 10, 2022 17:00

joernweissenborn reviewed Mar 12, 2022

View reviewed changes

glotaran/model/dataset_model.pyi Outdated Show resolved Hide resolved

s-weigand and others added 5 commits March 12, 2022 16:42

✨ Initial draft for a simplified specification of guidance spectra

99b5378

Co-authored-by: Jörn Weißenborn <[email protected]> Co-authored-by: Joris Snellenburg <[email protected]>

♻️🩹 Renamed overwrite_{model|global}_dimension to force_{model|global…

67361c0

…}_dimension So they won't conflict with the method which has the same name

🩹 Fixed tests

40628ae

♻️ Renamed attributes force_{model|global}_dimension back to

c320860

overwrite_{model|global}_dimension ♻️ Renamed methods overwrite_{model|global}_dimension to replace_{model|global}_dimension

♻️ Renamed replace_{model|global}_dimension methods

57c32a9

to set_{model|global}_dimension

s-weigand force-pushed the simplify-guidance branch from d0d354f to 57c32a9 Compare March 12, 2022 15:56

📚🩹 Fixed typos in docstrings

4c836d6

joernweissenborn reviewed Mar 12, 2022

View reviewed changes

glotaran/model/model.py Show resolved Hide resolved

s-weigand and others added 3 commits March 12, 2022 17:15

♻️ renamed overwrite_* -> override_*

7f565aa

♻️ Refactored by Sourcery

fed69bc

🩹 Fix wrong method renamed

a4df8f9

s-weigand closed this Mar 25, 2022

s-weigand mentioned this pull request Mar 25, 2022

✨Make guidance spectra a builtin megacomplex #1027

Closed

s-weigand reopened this Mar 26, 2022

sourcery-ai bot mentioned this pull request Mar 26, 2022

✨Make guidance spectra a first class citizen (Sourcery refactored) #1028

Closed

s-weigand closed this Mar 26, 2022

s-weigand deleted the simplify-guidance branch April 2, 2022 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨Make guidance spectra a first class citizen #983

✨Make guidance spectra a first class citizen #983

s-weigand commented Jan 30, 2022 •

edited

Loading

github-actions bot commented Jan 30, 2022

sonarqubecloud bot commented Jan 30, 2022

joernweissenborn left a comment

github-actions bot commented Jan 30, 2022 •

edited

Loading

codecov bot commented Jan 30, 2022 •

edited

Loading

jsnel left a comment

s-weigand commented Mar 25, 2022 •

edited

Loading

sourcery-ai bot commented Mar 26, 2022

sonarqubecloud bot commented Mar 26, 2022

s-weigand commented Mar 26, 2022

✨Make guidance spectra a first class citizen #983

✨Make guidance spectra a first class citizen #983

Conversation

s-weigand commented Jan 30, 2022 • edited Loading

Change summary

Checklist

Closes issues

github-actions bot commented Jan 30, 2022

sonarqubecloud bot commented Jan 30, 2022

joernweissenborn left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 30, 2022 • edited Loading

codecov bot commented Jan 30, 2022 • edited Loading

Codecov Report

jsnel left a comment

Choose a reason for hiding this comment

s-weigand commented Mar 25, 2022 • edited Loading

sourcery-ai bot commented Mar 26, 2022

Sourcery Code Quality Report

Legend and Explanation

sonarqubecloud bot commented Mar 26, 2022

s-weigand commented Mar 26, 2022

s-weigand commented Jan 30, 2022 •

edited

Loading

github-actions bot commented Jan 30, 2022 •

edited

Loading

codecov bot commented Jan 30, 2022 •

edited

Loading

s-weigand commented Mar 25, 2022 •

edited

Loading