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

Check more locations for dataset file and fail if comparison files mi… #348

Merged
merged 11 commits into from
Jan 7, 2021

Conversation

JonathanTripp
Copy link
Contributor

Searching for "dataset.csv" and "metrics.csv" used ValueError in the except block, but Azure ML now throws UserErrorException
if the file does not exist, so add that to the list to check.

Also, if the files are missing and they are expected then raise a new Exception and do not silently fail in the MLRunner.

Please follow the guidelines for PRs contained here. 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.
  • 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.

@JonathanTripp JonathanTripp linked an issue Jan 4, 2021 that may be closed by this pull request
Comment on lines 205 to 207
# noinspection PyUnresolvedReferences
if path is not None and not path.exists():
logging.warning(" ... but it does not exist")
Copy link
Contributor

Choose a reason for hiding this comment

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

this whole block from line 202 onwards, not sure this adds much value. Do we not yet have all the information in the logs, from line 191 and 179? Also, in which cases is download__outputs_from_run going to return something that is not None, but the file does not actually exist? This would simplify the checks.

Tests/AfterTraining/test_after_training.py Show resolved Hide resolved
@ant0nsc ant0nsc marked this pull request as ready for review January 6, 2021 20:49
@JonathanTripp JonathanTripp merged commit c54a728 into master Jan 7, 2021
@JonathanTripp JonathanTripp deleted the v-jontri/model_comparison branch January 7, 2021 11:02
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.

Model comparision is broken
3 participants