Skip to content

Conversation

skirmer
Copy link

@skirmer skirmer commented Sep 30, 2020

A proposed solution to #746. I ran tests like this to confirm it works across expected values:

for i in np.arange(0.01, 1.0, 0.0001):
    print(round(1-i, 6))

Base automatically changed from master to main February 2, 2021 03:43
@jsignell
Copy link
Member

This seems like a fine and concise solution to me.


if train_size is None and test_size is not None:
train_size = 1 - test_size
train_size = round(1 - test_size, 6)
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps, the number 6 could be promoted to a named variable to avoid copy-and-paste bugs upon possible updates to this.

Also, I think for some users, the value 6 wouldn't be as effective, as it would for others. Perhaps, this warrants a consideration for introducing a configurable parameter. If such parameter is absent, some default value (eg, 6, as it has been the choice here) could be used as a fallback.

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