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

Use Default::default instead of new in traits #230

Open
1 task done
morenol opened this issue Nov 4, 2022 · 2 comments · May be fixed by #231
Open
1 task done

Use Default::default instead of new in traits #230

morenol opened this issue Nov 4, 2022 · 2 comments · May be fixed by #231

Comments

@morenol
Copy link
Collaborator

morenol commented Nov 4, 2022

I'm submitting a

  • improvement.

Current Behaviour:

Currently, traits in https://github.com/smartcorelib/smartcore/blob/development/src/api.rs#L24 requires implementation of fn new() -> Self;, I think that instead we should rely on Default::default,

Do you want to work on this issue?

Yes

Any thoughts?
@morenol morenol changed the title Use Default::default instead of new in trait Use Default::default instead of new in traits Nov 4, 2022
@Mec-iS
Copy link
Collaborator

Mec-iS commented Nov 4, 2022

I had to implement the new() method as it looked like the best way of passing the estimators to cross_validate and cross_validate_predict. If you find a better way would be happy to go for it.

@morenol
Copy link
Collaborator Author

morenol commented Nov 4, 2022

gotcha, I think that we can use generics to define the type that we need to use instead of passing a blank parameter that is not used

@morenol morenol linked a pull request Nov 4, 2022 that will close this issue
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: do not require an empty struct in cross_validate to define … morenol/smartcore
2 participants
@Mec-iS @morenol