Markers/wrap up #10062
Markers/wrap up #10062
Conversation
rasa/cli/evaluate.py
Outdated
| @@ -120,30 +124,32 @@ def _run_markers( | |||
| strategy: Strategy to use when selecting trackers to extract from. | |||
| config: Path to the markers definition file to use. | |||
| output_filename: Path to write out the extracted markers. | |||
| stats_file: (Optional) Path to write out statistics about the extracted | |||
| markers. | |||
| stats_file_prefix: (Optional) Prefix of paths to write out statistics about the | |||
There was a problem hiding this comment.
I think this is a bit unclear since a path prefix is a different thing to what we're talking about here (a path, with a prefix to be appended to a file name)
There was a problem hiding this comment.
any suggestion for a nice name? :/
There was a problem hiding this comment.
I think the name is fine, but the docs should maybe read differently - it's a bit complicated because this is bundling two separate things in, a path, and the name prefix. Maybe something like "The path where stats files should be stored, and a prefix used to construct the stats file names" or something?
There was a problem hiding this comment.
Tried to add that to both the docstr and the help text with an example. A bit lengthy now but should be clearer now?
| session_stats_file=_append_suffix(stats_file_prefix, STATS_SESSION_SUFFIX), | ||
| overall_stats_file=_append_suffix(stats_file_prefix, STATS_OVERALL_SUFFIX), | ||
| ) | ||
| except (FileExistsError, NotADirectoryError) as e: |
| contained in a directory that does not exist | ||
| """ | ||
| # Check files and folders before doing the costly swipe over the trackers: | ||
| for path in [session_stats_file, overall_stats_file, output_file]: |
There was a problem hiding this comment.
So I put this in the CLI because I figured it would let us have a flag to allow the user to force an overwrite, do we want to bother adding in that feature right now or put it on the list of future enhancements?
There was a problem hiding this comment.
I like the idea :D (had also put an overwrite parameter to the stats :D) - But since it wasn't in the list of requirements, let's just ignore it for now
* connect stats and cli; rm unused types from marker.py; ... * rm unused schemata * try to improve docstr/help * Fix stats CLI command inversion * fix help test (nargs=? -> stats file prefix is optional) Co-authored-by: Matthew Summers <m.summers@rasa.com>
Proposed changes:
Status (please check what you already did):
black(please check Readme for instructions)