Skip to content

[Meta Issue] Changing defaults #4749

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

Closed
najeeb-kazmi opened this issue Jan 31, 2020 · 4 comments
Closed

[Meta Issue] Changing defaults #4749

najeeb-kazmi opened this issue Jan 31, 2020 · 4 comments
Assignees
Labels
API Issues pertaining the friendly API lightgbm Bugs related lightgbm P1 Priority of the issue for triage purpose: Needs to be fixed soon. ranking Bugs related ranking tasks

Comments

@najeeb-kazmi
Copy link
Member

First of all, is this a breaking change? @terrajobst suggested it is an acceptable breaking change #2305 (comment), @eerhardt commented that is was decided that changing defaults is acceptable #2305 (comment)

cc: @harishsk @justinormont @gvashishtha @eerhardt

@najeeb-kazmi
Copy link
Member Author

Assigning @gvashishtha for triage.

@justinormont
Copy link
Contributor

@justinormont correct me if I am wrong: was the default weighting TF-IDF in TLC? It is currently TF

The default text recipe in TLC uses TF. I believe the current default in ML․NET is also TF, so no changes needed for that.

@eerhardt
Copy link
Member

Changing a default parameter value is an acceptable change. Just be sure you still support the old default value - for example if the old default was ‘null’ and you change it to a non-null default, you still should support ‘null’ values.

@harishsk harishsk added the P1 Priority of the issue for triage purpose: Needs to be fixed soon. label Feb 3, 2020
@harishsk harishsk added ranking Bugs related ranking tasks lightgbm Bugs related lightgbm labels Apr 29, 2020
@michaelgsharp michaelgsharp self-assigned this Jul 1, 2020
@michaelgsharp
Copy link
Member

With the merging of PRs #5248, #5258, and #5290, this issue has been resolved. Closing this issue.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Issues pertaining the friendly API lightgbm Bugs related lightgbm P1 Priority of the issue for triage purpose: Needs to be fixed soon. ranking Bugs related ranking tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants