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

Fix {network, forest}.py default overloading #324

Merged
merged 5 commits into from
Oct 19, 2020
Merged

Conversation

levinwil
Copy link
Collaborator

Reference issue

Closes Issue #322

Type of change

Bug

What does this implement/fix?

Previously, we used None to instruct the usage of default parameters when calling 'add_{task, transformer}'. But, this has undesirable effects, such as the inability to add a new forest which trains to purity (unless the default value for the LLCF is None, which is not all the time). As such, we now use the string 'default' to indicate the usage of default parameters.

Additional information

@levinwil levinwil changed the base branch from main to staging October 19, 2020 16:12
@jdey4
Copy link
Collaborator

jdey4 commented Oct 19, 2020

Could you add a parameter to add_task as n_estimtaors? I need that for the experiments. I did this in a branch that has now merge conflicts.

@levinwil
Copy link
Collaborator Author

@jdey4 Ya sure but I'll do that in another PR. That way, each PR addresses one and only one fix.

@levinwil
Copy link
Collaborator Author

@jdey4 Can you please submit a feature request issue for that, so I remember to do so?

@levinwil levinwil changed the title Fixed {network, forest}.py default overloading Fix {network, forest}.py default overloading Oct 19, 2020
@jdey4 jdey4 merged commit 5e89bd1 into staging Oct 19, 2020
@levinwil levinwil mentioned this pull request Oct 19, 2020
@levinwil levinwil deleted the default_overload_fix branch October 19, 2020 17:21
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