Skip to content

Conversation

@m-philipps
Copy link
Collaborator

When reading in parameter tables using parameters.get_parameter_df:

  • check all parameter tables (not just those resulting from several parameter files) for contradicting parameter definitions
  • remove the removal of duplicate parameters, will instead raise ValueError
  • ensure that parameters that only differ by their parameterId are recognized as distinct and not dropped (fix Parameters dropped when using subset parameter files #154 )
  • adjust/extend test_parameters.py accordingly

@m-philipps m-philipps requested a review from dilpath June 14, 2022 08:27
Copy link
Member

@dilpath dilpath left a comment

Choose a reason for hiding this comment

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

👍

Thanks for fixing this, and the nice tests!

except KeyError as e:
raise KeyError(
f"Parameter table missing mandatory field {PARAMETER_ID}.") from e
_check_for_contradicting_parameter_definitions(parameter_df)
Copy link
Member

Choose a reason for hiding this comment

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

As the drop_duplicates call is removed, the duplicated parameter definitions may be consistent (e.g. your test raises an error for redundance too), so this check is now just about duplicated parameter IDs.

The logic for this should now be equivalent to petab.lint.assert_unique_parameter_ids. Would be good if that could be reused here, but fine as is, since the circular dependency would need to be resolved.

Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
@m-philipps m-philipps merged commit 9967ce1 into develop Jun 14, 2022
@m-philipps m-philipps deleted the fix_154 branch June 14, 2022 09:28
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.

3 participants