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

Improve documentation for validate_holdout_datasets #3270

Closed
dsherry opened this issue Jan 21, 2022 · 2 comments · Fixed by #3278
Closed

Improve documentation for validate_holdout_datasets #3270

dsherry opened this issue Jan 21, 2022 · 2 comments · Fixed by #3278
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@dsherry
Copy link
Contributor

dsherry commented Jan 21, 2022

After #3208 the helper validate_holdout_datasets is only used in test_gen_utils.py. Let's delete it unless there's a justifiable reason to keep it. If we do keep it, let's improve the docstring since its currently vague: "Validate the holdout datasets match our expectations."

@dsherry dsherry added the refactor Work being done to refactor code. label Jan 21, 2022
@freddyaboulton
Copy link
Contributor

freddyaboulton commented Jan 24, 2022

Our users use this function for time series problems. So let's not delete it and instead improve the docstring and maybe add it to the time series user guide. Renaming to reflect this.

@freddyaboulton freddyaboulton changed the title Remove unused time series validation method validate_holdout_datasets Improve documentation for validate_holdout_datasets Jan 24, 2022
@dsherry
Copy link
Contributor Author

dsherry commented Jan 25, 2022

@freddyaboulton @ParthivNaresh 👍 SGTM!

@dsherry dsherry added documentation Improvements or additions to documentation and removed refactor Work being done to refactor code. labels Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants