Kcyganik success msg closes #109 #125
Conversation
|
For now, it looks good, but it doesn't have a message about metric scores. Do we want it?
|
kordc
force-pushed
the
kcyganik-success-msg
branch
from
2f9afda to
f97cf34
Compare
|
Now it is better: |
art/experiment/Experiment.py
Outdated
| for step in self.steps: | ||
| self.metric_calculator.compile(step["skipped_metrics"]) | ||
| step, checks = step["step"], step["checks"] | ||
|
|
||
| self.state.current_step = step | ||
|
|
||
| if not self.check_if_must_be_run(step, checks): | ||
| steps_status[step.get_full_step_name()] = ("Skipped", step.get_results()) |
There was a problem hiding this comment.
Use some @dataclasses for this as done in cheks.py. This will be much more readable then just tuple
art/experiment/Experiment.py
Outdated
| except Exception as e: | ||
| steps_status[step.get_full_step_name()] = ("Failed", step.get_results()) | ||
| print(f"Step {step.get_full_step_name()} failed with error: {str(e)}") | ||
| continue |
There was a problem hiding this comment.
We don't want to continue here, it's the philosophy of art. We want to either reraise the exception or just break from the loop. So I'd remove this print and maybe add raised exception to dataclass of step_status
There was a problem hiding this comment.
@kordc still rest of steps will be run.
kordc
force-pushed
the
kcyganik-success-msg
branch
from
f97cf34 to
459514b
Compare
|
the newest commit closes #108 |
|
Approve, but If i remember correctly we wanted to save state into a file, so maybe we can do it here...? |
done |
closes #109