Skip to content

SciML extension: add linters - #505

Open
dilpath wants to merge 2 commits into
mainfrom
sciml_linters
Open

SciML extension: add linters#505
dilpath wants to merge 2 commits into
mainfrom
sciml_linters

Conversation

@dilpath

@dilpath dilpath commented Jul 22, 2026

Copy link
Copy Markdown
Member

Generated based on PEtab-dev/petab_sciml#69

@dilpath
dilpath requested review from BSnelling and m-philipps July 22, 2026 16:28
@dilpath
dilpath requested a review from a team as a code owner July 22, 2026 16:28
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.40704% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.33%. Comparing base (08489d2) to head (56f6fe5).

Files with missing lines Patch % Lines
petab/v2/extensions/sciml_lint.py 81.95% 18 Missing and 17 partials ⚠️
petab/v2/lint.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #505      +/-   ##
==========================================
+ Coverage   75.12%   75.33%   +0.20%     
==========================================
  Files          64       64              
  Lines        7161     7354     +193     
  Branches     1267     1327      +60     
==========================================
+ Hits         5380     5540     +160     
- Misses       1291     1309      +18     
- Partials      490      505      +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dweindl dweindl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +493 to +496
posterior = any(
p.prior_distribution is not None for p in problem.parameters
)
if posterior:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
posterior = any(
p.prior_distribution is not None for p in problem.parameters
)
if posterior:
if problem.has_map_objective:

?

(This only checks for priors on estimated parameters, though.)

Comment thread petab/v2/lint.py
Comment on lines +48 to +52
"CheckNeuralNetworkModel",
"CheckHybridizationTable",
"CheckSciMLConditionTable",
"CheckArrayDataFiles",
"CheckSciMLParameterTable",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to re-export them here? If so, I would prefer a SciML prefix in all of those; if not, let's remove them from here.

Comment thread petab/v2/lint.py
Comment on lines +1215 to +1216
except ImportError:
sciml_validation_tasks = default_validation_tasks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move sciml_validation_tasks to .extensions.sciml_lint or .extensions.sciml?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants