-
Notifications
You must be signed in to change notification settings - Fork 18
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
✨ Add clp guidance megacomplex #1029
Conversation
Benchmark is done. Checkout the benchmark result page. Benchmark diff v0.5.1 vs. mainParametrized benchmark signatures: BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)
Benchmark diff main vs. PRParametrized benchmark signatures: BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1029 +/- ##
=======================================
+ Coverage 86.9% 87.1% +0.2%
=======================================
Files 99 101 +2
Lines 5314 5377 +63
Branches 980 995 +15
=======================================
+ Hits 4618 4687 +69
+ Misses 544 536 -8
- Partials 152 154 +2 ☔ View full report in Codecov by Sentry. |
be0a643
to
b3f5d6b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor nitpicks about error messages and backward compatibility with the ascii
format.
Might also be nice to factor the creation out in a separate function so we can use it on a dataset from file basis (e.g. reading legacy project ascii
files from TMP
and TIMP
) 🤔
1301093
to
438eef2
Compare
Sourcery Code Quality Report❌ Merging this PR will decrease code quality in the affected files by 0.78%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
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! |
This is what the model would look like as dataset:
dataset1:
megacomplex:
- mc1
dataset2:
megacomplex:
- mc2
dataset_groups:
default:
link_clp: true
megacomplex:
mc1:
compartments:
- s1
- s2
rates:
- '1'
- '2'
type: decay-sequential
mc2:
dimension: time
target: s1
type: clp-guide |
Co-authored-by: Jörn Weißenborn <[email protected]>
438eef2
to
9a5d038
Compare
clp_label coordinate
The function can also be used for loaded result datasets
9a5d038
to
23d0360
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ready to merge now (reviewing my own changes 😝 )
@jsnel @ism200 For the usage guide of create_clp_guide_dataset
see the docs generated from the docstrings you can try it on binder (also works with ascii
just change the file extension)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed as ok, code wise, found some improvements to documentation here and there.
Minor improvements to documentation following code review on glotaran#1029. Fixed interrogate issues with invalid exclude argument in pyproject.toml Set ignore-semiprivate to true (for now) for interrogate (else coverage dips below 59) Added a docstring to improve coverage (as measured by interrogate)
Minor improvements to documentation following code review on glotaran#1029. Fixed interrogate issues with invalid exclude argument in pyproject.toml Set ignore-semiprivate to true (for now) for interrogate (else coverage dips below 59) Added a docstring to improve coverage (as measured by interrogate)
5e03926
to
ff1ada9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could do with some more documentation (doc strings), especially in the util function department, but code wise OK.
click>=8 breaks usage of glob patterns with interrogate Current docstring coverage is 65%
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
This PR adds the
clp-guide
megacomplex.A model with a clp guide looks like this:
Change summary
exclusive
attribute to megacomplex decorator which ensures that a megacomplex is the only one in a datasetClpGuideMegacomplex
Result.create_clp_guide_dataset
to create clp guides from resultsChecklist
Closes issues
closes #1027