biosimulators / Biosimulators_utils

Utilities for building standardized command-line interfaces for biosimulation software packages
https://docs.biosimulators.org/Biosimulators_utils
MIT License
4 stars 6 forks source link

fix(cellml): skip variables that do not have a valid observable #109

Closed bilalshaikh42 closed 2 years ago

bilalshaikh42 commented 2 years ago

this change skips over variables in the cellML that have been labeled as "public_interface"="in" as these are not exposed by OpenCor after simulation. No information is lost from the reports, as these variables have a connection to another component that contains the same variable, with the public interface set to "out". Accessing the variable through that component works fine. This addresses the issues seen in importing the physiome projects.

bilalshaikh42 commented 2 years ago

@jonrkarr I think some of the issues we are seeing with the invalid observables are addressed with this change and might be a distinct issue from https://github.com/opencor/opencor/issues/2612

jonrkarr commented 2 years ago

This seems fine. I don't think this is addresses https://github.com/opencor/opencor/issues/2612.

sonarcloud[bot] commented 2 years ago

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
66.7% 66.7% Duplication

codecov[bot] commented 2 years ago

Codecov Report

Merging #109 (8a1b0cb) into dev (a152d78) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##              dev     #109   +/-   ##
=======================================
  Coverage   96.62%   96.62%           
=======================================
  Files          87       87           
  Lines        9321     9326    +5     
=======================================
+ Hits         9006     9011    +5     
  Misses        315      315           
Flag Coverage Δ
unittests 96.62% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
biosimulators_utils/model_lang/cellml/utils.py 100.00% <100.00%> (ø)

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 a152d78...8a1b0cb. Read the comment docs.

bilalshaikh42 commented 2 years ago

@jonrkarr I changed this to keep the default behavior but added a flag to skip the non-observable values. That way the original tests/behavior is preserved, and I added a test case for when the flag is set to true. This should be good to merge, but let me know if I should just have the method skip over the non-observable variables by default

bilalshaikh42 commented 2 years ago

Also, I am not an admin on the biosimulators organization for sonar cloud, so I cannot configure the scanning to ignore the tests files for duplication errors. If you add me I can make the relevant changes, or we can just ignore it.

jonrkarr commented 2 years ago

Also, I am not an admin on the biosimulators organization for sonar cloud, so I cannot configure the scanning to ignore the tests files for duplication errors. If you add me I can make the relevant changes, or we can just ignore it.

You're already an owner of the organization. I don't see permissions for SonarCloud specifically. Where can I configure that?

bilalshaikh42 commented 2 years ago

https://sonarcloud.io/organizations/biosimulators/members should have an option to "configure synchronization" which will let you sync the permissions to the github organization