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.

Sign up for GitHub

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

Jump to bottom

save_dir for pt.Experiment #163 #247

Merged
merged 6 commits into from
Dec 9, 2021
Merged

save_dir for pt.Experiment #163 #247

merged 6 commits into from
Dec 9, 2021

Conversation

cmacdonald
Copy link
Contributor

No description provided.

@cmacdonald
Copy link
Contributor Author

Test cases will be added later

seanmacavaney
seanmacavaney reviewed Dec 8, 2021
View reviewed changes
pyterrier/io.py Outdated Show resolved Hide resolved
pyterrier/io.py Outdated Show resolved Hide resolved
pyterrier/pipelines.py Outdated Show resolved Hide resolved
pyterrier/pipelines.py Show resolved Hide resolved
pyterrier/pipelines.py Outdated Show resolved Hide resolved
pyterrier/pipelines.py Show resolved Hide resolved
seanmacavaney
seanmacavaney approved these changes Dec 9, 2021
View reviewed changes
pyterrier/pipelines.py Outdated Show resolved Hide resolved
@cmacdonald
Copy link
Contributor Author

ok, what about the names save_dir, save ="overwrite" or save="reuse" ? I kind of think the latter should be save_mode, but it seems unnecessary

@seanmacavaney
Copy link
Collaborator

ok, what about the names save_dir, save ="overwrite" or save="reuse" ? I kind of think the latter should be save_mode, but it seems unnecessary

"save" could be confused with a boolean argument that just indicates whether it should save or not. Probably safer to use "save_mode".

@cmacdonald
Copy link
Contributor Author

added documentation, can you review that too, then this is done.

@cmacdonald
Copy link
Contributor Author

(failures are unrelated)

@cmacdonald cmacdonald merged commit 5a69676 into master Dec 9, 2021
@cmacdonald cmacdonald deleted the exp_save branch December 9, 2021 18:21
@cmacdonald cmacdonald mentioned this pull request Dec 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanmacavaney seanmacavaney seanmacavaney approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cmacdonald @seanmacavaney