v2: Allow applying multiple conditions simultaneously

[dweindl]

Adapt to PEtab-dev/PEtab#616, which allows providing multiple conditions for the same experiment / timepoint.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 74.17%. Comparing base (b43f5b8) to head (ba1996f).

Files with missing lines	Patch %	Lines
petab/v2/lint.py	85.71%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #373      +/-   ##
===========================================
+ Coverage    74.08%   74.17%   +0.08%     
===========================================
  Files           58       58              
  Lines         6221     6234      +13     
  Branches      1078     1084       +6     
===========================================
+ Hits          4609     4624      +15     
+ Misses        1206     1204       -2     
  Partials       406      406              

☔ View full report in Codecov by Sentry.
📢 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.

[dilpath]

Looks good! Nothing that needs to be changed.

[dilpath]

Might need to add a dropna

Suggested change

	for timepoint in cur_exp_df[C.TIME].unique():
	condition_ids = [
	cid
	for cid in cur_exp_df.loc[
	cur_exp_df[C.TIME] == timepoint, C.CONDITION_ID
	]
	if not pd.isna(cid)
	]
	for timepoint, cur_exp_time_df in cur_exp_df.groupby(C.TIME):
	condition_ids = cur_exp_time_df[C.CONDITION_ID].unique()

[dweindl]

I wanted to avoid the overhead of the additional groupy / creating the additional dataframes. Usually there will only be a single condition ID.

I'd avoid the unique otherwise this will hide errors in case of duplicated conditions.

[dilpath]

creating the additional dataframes

Ah, you're right, I thought it was some view instead

I'd avoid the unique

Yes, not sure why I did that...

No issue with your code but I find the groupby faster to understand.
Independently of this, again this might be something that tools want in a library function

[dilpath]

Is this useful for tools? I guess tools might like some problem.condition_table.get_updates(condition_id) that provides a dictionary of updates to be applied. Then the keys can be used here.

[dweindl]

If it's already in a dict, the keys will be unique and we can't check for duplicates any. It works if we do the checking every time in such a function.
I think something like that will come when progressing with the importer.

[dilpath]

I rather meant like this. It checks all conditions at once though, rather than one at a time, but there's no issue with identifying duplicates.

from collections import Counter
from itertools import chain

all_updates = [
    problem.condition_table.get_updates(condition_id)
    for condition_id in period.condition_ids
]
duplicates = {
    target_id
    for target_id, count in Counter(chain.from_iterable(all_updates)).items()
    if count > 1
}

Or a method that provides all_updates as a condition_id => updates dict.

[dilpath]

Not that it simplifies the code here, just that it might be useful to tools. But fine to keep as is until a tool dev requests something...

[dilpath]

Same thing for here and below, is it useful to add this to the classes? Looks like a lot of additional interface though so fine as is

problem.experiment_table.get_condition_ids()
problem.experiment_table.get_experiment(experiment_id).get_condition_ids()

[dweindl]

I am not sure yet. I think, usually one only needs the conditions for a single period, not for the full experiment at once.

[dilpath]

🤯

Didn't know an SBML test model could be specified so simply.

[dweindl]

💪