Lint mapping table

[m-philipps]

• core: Update Fields/Annotations to match the PEtab v2 docs
• add a lint check for the mapping table
• Change the logic in petab.v2.lint.get_valid_parameters_for_parameter_table: add mapping table petabEntityId regardless of whether the corresponding modelEntityId would be valid (was L937->L1036)
• add a test for mapping table linting

[m-philipps]

Not necessary since this is collected as a set and the condition_targets are removed further down.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 80.55556% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.41%. Comparing base (3363e85) to head (72d7a29).

Files with missing lines	Patch %	Lines
petab/v2/lint.py	79.41%	3 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #480      +/-   ##
==========================================
+ Coverage   75.29%   75.41%   +0.11%     
==========================================
  Files          62       62              
  Lines        6946     6975      +29     
  Branches     1229     1240      +11     
==========================================
+ Hits         5230     5260      +30     
+ Misses       1245     1239       -6     
- Partials      471      476       +5     

☔ 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]

Thanks. Almost there, I think.

[dweindl]

By duplicates, you mean petab_id == model_id, not multiple rows with the same petab_id or model_id, right?
It's not super clear from the specs whether the latter would be allowed or not.

[BSnelling]

Yes, I've reworded to comment to make this clearer.

[dweindl]

This might add all kinds of entities to the allowed-in-parameter-table dict. Species, observables, experiments, ...
For the currently supported model types (SBML, PySB), I don't think there can be a situation where the mapping table would contribute additional values allowed in the parameter table. For a model type where this could happen, we'd need some API for accessing parameters that are allowed in the parameter table if they had proper IDs. This doesn't exist yet. So either we need to add it now, or completely ignore the mapping table in this function. (The same holds for get_required_parameters_for_parameter_table below.)

[BSnelling]

a situation where the mapping table would contribute additional values allowed in the parameter table

SciML problems have this use case don't they? For example, we'd define net_ps as a PEtab id for the network parameters net1.parameters in the mapping table, and then also have net_ps specified in the parameters table with its bounds etc.

[dweindl]

Quite possible, I am not sufficiently familiar with petab-sciml. In that case, I would ignore it for now, revert the changes in get_valid_parameters_for_parameter_table, and address it after this PR and #482. It will probably require a bit more thought and changes.

[dweindl]

The same for get_required_parameters_for_parameter_table. As far as I can tell, with the currently (pre-sciml) supported models, the mapping table won't affect what's allowed or required in the parameter table.

[BSnelling]

I have removed all the mapping table related changes from both functions and updated the unit test.

[dweindl]

Thanks, looks good to me, except for that duplication!

[dilpath]

Thanks a lot. Looks good!

[dilpath]

Where does it do this? There is no ValidationError with a message like "Unused mapping table PEtab IDs: ...."

[BSnelling]

Good point, it doesn't do this yet as I'm planning to handle mapping/parameter table interaction in the sciml branch. I've removed from the docstring.