Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support custom results/details path #451

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

albertvillanova
Copy link
Member

@albertvillanova albertvillanova commented Dec 16, 2024

Support custom results/details path.

This PR adds the optional output_path parameter to EvaluationTracker.save_results and EvaluationTracker.save_details:

  • If None, same behavior as before.
  • If passed, use this as the results output path.

Also, it makes optional the EvaluationTracker.output_dir attribute:

  • If set, use it as the common dir for output results/details (as before).
  • If None, the output results and details can be fully customized and no longer need of a common dir for both.

TODO:

  • Docstrings

@HuggingFaceDocBuilderDev
Copy link
Collaborator

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

self, date_id: str | None = None, results_dict: dict | None = None, output_path: str | None = None
):
if output_path:
fs, output_path = url_to_fs(output_path)
Copy link
Member

Choose a reason for hiding this comment

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

so it will be overiding output_dir, this isn't very clear. Users should be able to set the output_path in a coherent way with results and details.

Copy link
Member Author

@albertvillanova albertvillanova Dec 17, 2024

Choose a reason for hiding this comment

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

I agree. The output_dir attribute is ignored if the user passes the output_path parameter.

One could make output_dir optional:

  • If passed, current behavior.
  • If None, then needs an output_path for all save_ methods.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or simpler: one could use output_dir as the base dir for the passed output_path (considered this as relative to output_dir.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also note that the previous approach forces the user to save results and details under a common output_dir. What about if this is not the case?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have made output_dir optional.

@albertvillanova albertvillanova marked this pull request as draft December 17, 2024 11:32
@albertvillanova albertvillanova changed the title Support custom results path Support custom results/details path Dec 18, 2024
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