Serveral refactorings in `glotaran.Parameter` #910

joernweissenborn · 2021-11-20T16:29:52Z

This PR streamlines the to and from dataframe methods of parameter group and ensures all parameter properties are included in the dataframe.

Change summary

Added methods as_dict and from_dict to Parameter
Added methods to_parameter_dict_list and from_parameter_dict_list to ParameterGroup
Refactored to_dataframe and from_dataframe to use new methods
Refactored parameter tests
Load standard error from df
save standard error to parameter df
- to_datafame has a switch as_optimized now which defaults to true, a later pr must adapt the io API
Added Parameter.markdown as basis for follow up PR
- This NOT used ParameterGroup to markdown. It is intended to be used by Model.markdown

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 #902
u

…rendering.py

github-actions · 2021-11-20T16:30:05Z

👈 Launch a binder notebook on branch joernweissenborn/pyglotaran/fix/savestderr

github-actions · 2021-11-20T16:34:02Z

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

Benchmark diff v0.5.0rc1 vs. main

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)


All benchmarks:

       before           after         ratio
     [d05c042a]       [4168edd4]
     <v0.5.0rc1>                 
         60.9±1ms       64.9±0.9ms     1.07  BenchmarkOptimize.time_optimize(False, False, False)
        90.6±30ms         69.6±4ms    ~0.77  BenchmarkOptimize.time_optimize(False, False, True)
         63.2±5ms         63.8±2ms     1.01  BenchmarkOptimize.time_optimize(False, True, False)
         100±30ms        73.0±20ms    ~0.73  BenchmarkOptimize.time_optimize(False, True, True)
         75.4±1ms         78.3±4ms     1.04  BenchmarkOptimize.time_optimize(True, False, False)
        84.4±40ms        84.3±20ms     1.00  BenchmarkOptimize.time_optimize(True, False, True)
         74.9±1ms         76.3±1ms     1.02  BenchmarkOptimize.time_optimize(True, True, False)
         134±50ms        87.8±40ms    ~0.66  BenchmarkOptimize.time_optimize(True, True, True)
             197M             196M     1.00  IntegrationTwoDatasets.peakmem_optimize
+      1.79±0.06s       2.09±0.03s     1.17  IntegrationTwoDatasets.time_optimize

Benchmark diff main vs. PR

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)


All benchmarks:

       before           after         ratio
     [2d44c75e]       [4168edd4]
         64.7±2ms       64.9±0.9ms     1.00  BenchmarkOptimize.time_optimize(False, False, False)
         73.3±5ms         69.6±4ms     0.95  BenchmarkOptimize.time_optimize(False, False, True)
       63.8±0.7ms         63.8±2ms     1.00  BenchmarkOptimize.time_optimize(False, True, False)
         73.0±9ms        73.0±20ms     1.00  BenchmarkOptimize.time_optimize(False, True, True)
       75.9±0.6ms         78.3±4ms     1.03  BenchmarkOptimize.time_optimize(True, False, False)
        89.5±30ms        84.3±20ms     0.94  BenchmarkOptimize.time_optimize(True, False, True)
         77.4±2ms         76.3±1ms     0.99  BenchmarkOptimize.time_optimize(True, True, False)
         175±50ms        87.8±40ms    ~0.50  BenchmarkOptimize.time_optimize(True, True, True)
             195M             196M     1.01  IntegrationTwoDatasets.peakmem_optimize
       2.10±0.08s       2.09±0.03s     1.00  IntegrationTwoDatasets.time_optimize

…ons.

codecov · 2021-11-21T13:38:15Z

Codecov Report

Merging #910 (4168edd) into main (2d44c75) will decrease coverage by 0.3%.
The diff coverage is 64.5%.

@@           Coverage Diff           @@
##            main    #910     +/-   ##
=======================================
- Coverage   85.1%   84.7%   -0.4%     
=======================================
  Files         85      85             
  Lines       4761    4790     +29     
  Branches     880     891     +11     
=======================================
+ Hits        4053    4060      +7     
- Misses       561     581     +20     
- Partials     147     149      +2

Impacted Files	Coverage Δ
glotaran/parameter/parameter.py	`91.2% <41.1%> (-7.5%)`	⬇️
glotaran/parameter/parameter_group.py	`88.2% <92.8%> (-1.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 2d44c75...4168edd. Read the comment docs.

glotaran/parameter/parameter_group.py

Fix typo

s-weigand

LGTM just some leftover nitpicks 👍

glotaran/parameter/parameter_group.py

glotaran/parameter/parameter.py

glotaran/parameter/test/test_parameter.py

glotaran/parameter/parameter.py

glotaran/parameter/parameter_group.py

sourcery-ai · 2021-11-21T20:49:17Z

Sourcery Code Quality Report

✅ Merging this PR will increase code quality in the affected files by 0.21%.

Quality metrics	Before	After	Change
Complexity	1.91 ⭐	1.60 ⭐	-0.31 👍
Method Length	49.20 ⭐	49.09 ⭐	-0.11 👍
Working memory	5.98 ⭐	6.03 ⭐	0.05 👎
Quality	77.72% ⭐	77.93% ⭐	0.21% 👍

Other metrics	Before	After	Change
Lines	2329	2477	148

Changed files	Quality Before	Quality After	Quality Change
glotaran/deprecation/modules/test/test_parameter_parameter_group.py	90.24% ⭐	90.24% ⭐	0.00%
glotaran/model/test/test_model.py	72.58% 🙂	72.58% 🙂	0.00%
glotaran/parameter/parameter.py	88.34% ⭐	85.18% ⭐	-3.16% 👎
glotaran/parameter/parameter_group.py	69.42% 🙂	75.68% ⭐	6.26% 👍
glotaran/parameter/test/test_parameter.py	78.54% ⭐	80.95% ⭐	2.41% 👍
glotaran/parameter/test/test_parameter_group.py	87.27% ⭐	74.28% 🙂	-12.99% 👎

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

File	Function	Complexity	Length	Working Memory	Quality	Recommendation
glotaran/parameter/parameter.py	Parameter.markdown	12 🙂	134 😞	11 😞	51.29% 🙂	Try splitting into smaller methods. Extract out complex expressions
glotaran/parameter/parameter_group.py	ParameterGroup.from_dataframe	14 🙂	134 😞	10 😞	51.47% 🙂	Try splitting into smaller methods. Extract out complex expressions
glotaran/model/test/test_model.py	test_fill	0 ⭐	243 ⛔	7 🙂	62.26% 🙂	Try splitting into smaller methods
glotaran/parameter/parameter.py	Parameter.__init__	1 ⭐	70 🙂	16 ⛔	63.36% 🙂	Extract out complex expressions
glotaran/parameter/parameter_group.py	ParameterGroup.__repr__	6 ⭐	72 🙂	12 😞	63.49% 🙂	Extract out complex expressions

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!

sonarcloud · 2021-11-21T20:49:40Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

jsnel

Reviewed ok.

s-weigand

LGTM 👍

joernweissenborn added 5 commits November 20, 2021 16:13

Added Parameter to and from dict methods.

ad18248

Moved markdown related parameter group tests to test_parameter_group_…

0064f88

…rendering.py

Refactored parameter tests.

4cc77d2

Added parameter to and from dictonionary list.

a76ad61

Added more extensive test for paramater csv.

02f5a9e

joernweissenborn requested a review from jsnel as a code owner November 20, 2021 16:29

joernweissenborn requested a review from s-weigand November 20, 2021 16:30

joernweissenborn self-assigned this Nov 20, 2021

joernweissenborn marked this pull request as draft November 20, 2021 16:46

joernweissenborn added 2 commits November 21, 2021 14:31

Added as_optimized to parameter(group) as dict and dataframe functi…

f6f5026

…ons.

Fixed deprecation test.

95aac8c

Added Parameter.markdown

ef65e56

sourcery-ai bot mentioned this pull request Nov 21, 2021

Serveral refactorings in glotaran.Parameter (Sourcery refactored) #911

Closed

joernweissenborn marked this pull request as ready for review November 21, 2021 16:31

joernweissenborn commented Nov 21, 2021

View reviewed changes

glotaran/parameter/parameter_group.py Outdated Show resolved Hide resolved

joernweissenborn and others added 7 commits November 21, 2021 18:07

Update glotaran/parameter/parameter_group.py

5e2d1e2

Fix typo

♻️ Refactored by Sourcery (less if nesting)

0d10f4b

🩹 Fixed renamed method get_group_for_parameter_by_label only in def

36123af

🧹 Removed redunent int since mypy sees int as subclass of float

bed48e8

♻️ Changed stderr type from 'float|None' to float and default to np.nan

9d5ea14

🩹🧪 Fixed tests due to changed rendering

9402e55

👌 Improved typing where dict and list were used w/o types

5172afa

s-weigand previously approved these changes Nov 21, 2021

View reviewed changes

jsnel reviewed Nov 21, 2021

View reviewed changes

glotaran/parameter/parameter.py Show resolved Hide resolved

jsnel reviewed Nov 21, 2021

View reviewed changes

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

jsnel reviewed Nov 21, 2021

View reviewed changes

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

jsnel reviewed Nov 21, 2021

View reviewed changes

glotaran/parameter/parameter_group.py Show resolved Hide resolved

s-weigand added 2 commits November 21, 2021 21:23

🧹👌 Fixed up left over tying issues in parameters

053e356

🧹 Fixed typo

6c418bb

s-weigand dismissed their stale review via 6c418bb November 21, 2021 20:25

🧹 Apply minor naming suggestions from code review

4168edd

jsnel self-requested a review November 21, 2021 21:32

jsnel approved these changes Nov 21, 2021

View reviewed changes

s-weigand approved these changes Nov 22, 2021

View reviewed changes

s-weigand merged commit f71a34b into glotaran:main Nov 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Serveral refactorings in `glotaran.Parameter` #910

Serveral refactorings in `glotaran.Parameter` #910

joernweissenborn commented Nov 20, 2021 •

edited

Loading

github-actions bot commented Nov 20, 2021

github-actions bot commented Nov 20, 2021 •

edited

Loading

codecov bot commented Nov 21, 2021 •

edited

Loading

s-weigand left a comment

sourcery-ai bot commented Nov 21, 2021

sonarcloud bot commented Nov 21, 2021

jsnel left a comment

s-weigand left a comment

Serveral refactorings in glotaran.Parameter #910

Serveral refactorings in glotaran.Parameter #910

Conversation

joernweissenborn commented Nov 20, 2021 • edited Loading

Change summary

Checklist

Closes issues

github-actions bot commented Nov 20, 2021

github-actions bot commented Nov 20, 2021 • edited Loading

codecov bot commented Nov 21, 2021 • edited Loading

Codecov Report

s-weigand left a comment

Choose a reason for hiding this comment

sourcery-ai bot commented Nov 21, 2021

Sourcery Code Quality Report

Legend and Explanation

sonarcloud bot commented Nov 21, 2021

jsnel left a comment

Choose a reason for hiding this comment

s-weigand left a comment

Choose a reason for hiding this comment

Serveral refactorings in `glotaran.Parameter` #910

Serveral refactorings in `glotaran.Parameter` #910

joernweissenborn commented Nov 20, 2021 •

edited

Loading

github-actions bot commented Nov 20, 2021 •

edited

Loading

codecov bot commented Nov 21, 2021 •

edited

Loading