Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Guard plot_cross_validation.save_outliers for institutionId and seriesId columns confirm with unit test #446

Merged
merged 7 commits into from
Apr 29, 2021

Conversation

dumbledad
Copy link
Contributor

@dumbledad dumbledad commented Apr 27, 2021

Fixing Issue #423

Overview:

Guarding the call to create_portal_query_for_outliers so that the required columns are present, and providing unit test that confirms the change.

Detail:

The functions save_outliers and create_portal_query_for_outliers in InnerEye.ML.cisualizers.lot_cross_validation
fail if the columns institutionId and seriesId are missing from the data. This PR

  • Augments create_portal_query_for_outliers so that it checks for the required columns and raises an informative ValueError if they are missing. The function description is updated accordingly.
  • Changes save_outliers in two ways. If the columns are missing
    • they are not used in the groupby which gleans the stats, and
    • the portal query is omitted.
  • Test data is provided to check that the revised save_outliers produces expected data when the columns are missing
  • The unit tests test_save_outliers and test_create_portal_query_for_outliers in Tests.ML.visualizers.test_plot_cross_validation are extended to exercise the new code with columns missing.

Checklist:

  • Ensure that your PR is small, and implements one change.
  • Add unit tests for all functions that you introduced or modified.
  • Run PyCharm's code cleanup tools on your Python files. (I'm using VS Code and have not done this.)
  • Link the correct GitHub issue for tracking.
  • Update the Changelog file: Describe your change in terms of
    Added/Changed/Removed/... in the "Upcoming" section.
  • When merging your PR, replace the default merge message with a description of your PR,
    and if needed a motivation why that change was required.

@dumbledad dumbledad self-assigned this Apr 27, 2021
@dumbledad dumbledad force-pushed the timregan/423-save_outliers-fails branch 2 times, most recently from 36e27e5 to a1a1274 Compare April 28, 2021 11:44
Tim Regan added 6 commits April 28, 2021 13:39
@dumbledad dumbledad force-pushed the timregan/423-save_outliers-fails branch from a5e60c5 to 131f393 Compare April 28, 2021 12:41
@dumbledad dumbledad marked this pull request as ready for review April 28, 2021 12:41
@dumbledad dumbledad enabled auto-merge (squash) April 28, 2021 14:14
@dumbledad dumbledad merged commit 9a7ac87 into main Apr 29, 2021
@dumbledad dumbledad deleted the timregan/423-save_outliers-fails branch April 29, 2021 14:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

save_outliers() fails if institutionId and seriesId undefined in dataset.csv
3 participants