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

Rename LearnerCrossfit.n_fits_ to LearnerCrossfit.n_splits_ #128

Merged
merged 2 commits into from
Oct 22, 2020

Conversation

j-ittner
Copy link
Member

This PR renames LearnerCrossfit.n_fits_ to LearnerCrossfit.n_splits_ for more consistent terminology: we refer to the elements of a crossfit as the splits not the fits.

@joerg-schneider
Copy link
Collaborator

@j-ittner
Should n_fits terminolgy remain here?:
https://github.com/BCG-Gamma/facet/blob/984e9cc94adc52a6d9c1181a525e95f135326cb7/src/facet/crossfit/_crossfit.py#L272-L274

Thinking of resize, there seem to be two concepts we call the same

  1. initially done fits/splits as defined via underlying CV
  2. projection to a subset of 1) to be selected for the new crossfit object – maybe calling the function argument to_n_splits or keep_n_splits makes that easier to grasp from the outside

@j-ittner
Copy link
Member Author

@j-ittner
Should n_fits terminolgy remain here?:

https://github.com/BCG-Gamma/facet/blob/984e9cc94adc52a6d9c1181a525e95f135326cb7/src/facet/crossfit/_crossfit.py#L272-L274

Thinking of resize, there seem to be two concepts we call the same

  1. initially done fits/splits as defined via underlying CV
  2. projection to a subset of 1) to be selected for the new crossfit object – maybe calling the function argument to_n_splits or keep_n_splits makes that easier to grasp from the outside

I should change this, well spotted!

@j-ittner j-ittner marked this pull request as draft October 22, 2020 10:52
@j-ittner j-ittner marked this pull request as ready for review October 22, 2020 11:09
@j-ittner j-ittner merged commit 91d4b06 into develop Oct 22, 2020
@mgelsm mgelsm deleted the feature/crossfit_n_splits_ branch October 22, 2020 16:38
@j-ittner j-ittner added this to the 1.0.1 milestone Mar 3, 2021
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.

2 participants