🩹 Remove inheriting from 'FileLoadableProtocol' #912

s-weigand · 2021-11-25T17:15:30Z

In python 3.9.7 (currently the only python 3.9 version on conda) there was a bug that ignores the init method of classes that inherit from a Protocol.

This lead to the following error (discovered by @jsnel )

TypeError: Protocols cannot be instantiated

Rather than restricting the usable python versions for typing sake, I will file an issue at mypy since the conformity with protocols should be picked up automatically.

Change summary

🩹 Removed inheriting from 'FileLoadableProtocol'

Checklist

✔️ Passing the tests (mandatory for all PR's)

In python 3.9.7 (currently the only python 3.9 version on conda) there was a bug that ignores the __init__ method of classes that inherit from a Protocol. Rather than restricting the usable python versions for typing sake, I will file an issue at mypy since the conformity with protocols should be picked up automatically.

sourcery-ai · 2021-11-25T17:15:44Z

Sourcery Code Quality Report

Merging this PR leaves code quality unchanged.

Quality metrics	Before	After	Change
Complexity	5.24 ⭐	5.24 ⭐	0.00
Method Length	40.22 ⭐	40.11 ⭐	-0.11 👍
Working memory	7.06 🙂	7.06 🙂	0.00
Quality	75.38% ⭐	75.38% ⭐	0.00%

Other metrics	Before	After	Change
Lines	1847	1841	-6

Changed files	Quality Before	Quality After	Quality Change
glotaran/model/model.py	70.82% 🙂	70.83% 🙂	0.01% 👍
glotaran/parameter/parameter_group.py	75.68% ⭐	75.69% ⭐	0.01% 👍
glotaran/parameter/parameter_history.py	94.62% ⭐	94.63% ⭐	0.01% 👍
glotaran/project/result.py	77.52% ⭐	77.52% ⭐	0.00%
glotaran/project/scheme.py	72.07% 🙂	72.08% 🙂	0.01% 👍
glotaran/project/test/test_dataclass_helpers.py	83.83% ⭐	83.83% ⭐	0.00%
glotaran/utils/io.py	75.07% ⭐	75.08% ⭐	0.01% 👍

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

File	Function	Complexity	Length	Working Memory	Quality	Recommendation
glotaran/utils/io.py	_load_datasets	15 🙂	156 😞	12 😞	44.60% 😞	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/project/scheme.py	Scheme.__post_init__	16 🙂	127 😞	11 😞	48.75% 😞	Try splitting into smaller methods. 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/model/model.py	Model.from_dict	12 🙂	162 😞	10 😞	50.19% 🙂	Try splitting into smaller methods. 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!

github-actions · 2021-11-25T17:15:45Z

👈 Launch a binder notebook on branch s-weigand/pyglotaran/fix-py3.9.7-bug

sonarcloud · 2021-11-25T17:16:19Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
1.7% Duplication

github-actions · 2021-11-25T17:19:35Z

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]       [38b0bd4a]
     <v0.5.0rc1>                 
       60.9±0.2ms         63.7±2ms     1.05  BenchmarkOptimize.time_optimize(False, False, False)
         67.1±2ms        73.4±40ms     1.09  BenchmarkOptimize.time_optimize(False, False, True)
       60.7±0.3ms       63.0±0.3ms     1.04  BenchmarkOptimize.time_optimize(False, True, False)
        70.1±30ms        70.1±10ms     1.00  BenchmarkOptimize.time_optimize(False, True, True)
       74.1±0.6ms       76.8±0.3ms     1.04  BenchmarkOptimize.time_optimize(True, False, False)
       80.0±0.3ms         140±40ms    ~1.75  BenchmarkOptimize.time_optimize(True, False, True)
       74.5±0.6ms       75.8±0.5ms     1.02  BenchmarkOptimize.time_optimize(True, True, False)
        85.0±40ms        82.9±30ms     0.98  BenchmarkOptimize.time_optimize(True, True, True)
             191M             198M     1.04  IntegrationTwoDatasets.peakmem_optimize
       1.79±0.06s        2.04±0.1s    ~1.14  IntegrationTwoDatasets.time_optimize

Benchmark diff main vs. PR

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)


All benchmarks:

       before           after         ratio
     [f71a34b7]       [38b0bd4a]
       62.6±0.3ms         63.7±2ms     1.02  BenchmarkOptimize.time_optimize(False, False, False)
         67.9±1ms        73.4±40ms     1.08  BenchmarkOptimize.time_optimize(False, False, True)
         63.1±1ms       63.0±0.3ms     1.00  BenchmarkOptimize.time_optimize(False, True, False)
        68.9±20ms        70.1±10ms     1.02  BenchmarkOptimize.time_optimize(False, True, True)
       75.2±0.5ms       76.8±0.3ms     1.02  BenchmarkOptimize.time_optimize(True, False, False)
        83.7±30ms         140±40ms    ~1.68  BenchmarkOptimize.time_optimize(True, False, True)
         77.4±2ms       75.8±0.5ms     0.98  BenchmarkOptimize.time_optimize(True, True, False)
        94.8±40ms        82.9±30ms    ~0.87  BenchmarkOptimize.time_optimize(True, True, True)
             193M             198M     1.03  IntegrationTwoDatasets.peakmem_optimize
       2.05±0.04s        2.04±0.1s     0.99  IntegrationTwoDatasets.time_optimize

codecov · 2021-11-25T17:19:49Z

Codecov Report

Merging #912 (38b0bd4) into main (f71a34b) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main    #912     +/-   ##
=======================================
- Coverage   84.7%   84.5%   -0.2%     
=======================================
  Files         85      85             
  Lines       4790    4785      -5     
  Branches     891     891             
=======================================
- Hits        4060    4047     -13     
- Misses       581     589      +8     
  Partials     149     149

Impacted Files	Coverage Δ
glotaran/model/model.py	`85.5% <100.0%> (-0.1%)`	⬇️
glotaran/parameter/parameter_group.py	`88.1% <100.0%> (-0.1%)`	⬇️
glotaran/parameter/parameter_history.py	`79.1% <100.0%> (-0.5%)`	⬇️
glotaran/project/result.py	`85.2% <100.0%> (ø)`
glotaran/project/scheme.py	`92.6% <100.0%> (-0.1%)`	⬇️
glotaran/utils/io.py	`100.0% <100.0%> (ø)`
glotaran/typing/protocols.py	`0.0% <0.0%> (-100.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 f71a34b...38b0bd4. Read the comment docs.

joernweissenborn

lgtm

jsnel

lgtm

s-weigand requested review from joernweissenborn and jsnel as code owners November 25, 2021 17:15

s-weigand added the Priority: Critical Nobody can use the software label Nov 25, 2021

s-weigand added this to the v0.5.0 milestone Nov 25, 2021

joernweissenborn approved these changes Nov 25, 2021

View reviewed changes

jsnel approved these changes Nov 25, 2021

View reviewed changes

jsnel merged commit 979643a into glotaran:main Nov 25, 2021

jsnel deleted the fix-py3.9.7-bug branch November 25, 2021 22:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🩹 Remove inheriting from 'FileLoadableProtocol' #912

🩹 Remove inheriting from 'FileLoadableProtocol' #912

s-weigand commented Nov 25, 2021 •

edited

Loading

sourcery-ai bot commented Nov 25, 2021

github-actions bot commented Nov 25, 2021

sonarcloud bot commented Nov 25, 2021

github-actions bot commented Nov 25, 2021

codecov bot commented Nov 25, 2021 •

edited

Loading

joernweissenborn left a comment

jsnel left a comment

🩹 Remove inheriting from 'FileLoadableProtocol' #912

🩹 Remove inheriting from 'FileLoadableProtocol' #912

Conversation

s-weigand commented Nov 25, 2021 • edited Loading

Change summary

Checklist

sourcery-ai bot commented Nov 25, 2021

Sourcery Code Quality Report

Legend and Explanation

github-actions bot commented Nov 25, 2021

sonarcloud bot commented Nov 25, 2021

github-actions bot commented Nov 25, 2021

codecov bot commented Nov 25, 2021 • edited Loading

Codecov Report

joernweissenborn left a comment

Choose a reason for hiding this comment

jsnel left a comment

Choose a reason for hiding this comment

s-weigand commented Nov 25, 2021 •

edited

Loading

codecov bot commented Nov 25, 2021 •

edited

Loading