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

Simplify eval args #1828

Merged
merged 3 commits into from
Oct 18, 2022
Merged

Simplify eval args #1828

merged 3 commits into from
Oct 18, 2022

Conversation

AdityaSoni19031997
Copy link
Contributor

@AdityaSoni19031997 AdityaSoni19031997 commented Oct 16, 2022

Description

The changes has been made to simplify the arguments required for the evaluation of @k metrics. It was found that col_rating column wasn't used by the these methods, hence the function signature could be simplified for better user experience.

Pytest Results

>>>pytest tests/unit/recommenders/evaluation/test_python_evaluation.py

platform darwin -- Python 3.8.14, pytest-7.1.3, pluggy-1.0.0
rootdir: /Users/asoni9/Desktop/recommenders
plugins: hypothesis-6.56.2, mock-3.10.0, rerunfailures-10.2, cov-4.0.0
collected 30 items                                                                                                                                                                        

tests/unit/recommenders/evaluation/test_python_evaluation.py ..............................  

==== 30 passed, 8 warnings in 0.84s ====

Related Issues

#1737

References

Not Applicable

Checklist:

  • I have followed the contribution guidelines and code style for this project.
  • [] I have added tests covering my contributions.
  • [] I have updated the documentation accordingly.
  • This PR is being made to staging branch and not to main branch.

miguelgfierro and others added 2 commits October 10, 2022 17:45
Staging to main: optimizations splitters, NDCG, and update tests
Copy link
Collaborator

@miguelgfierro miguelgfierro left a comment

Choose a reason for hiding this comment

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

I think this is really good. I just added some format suggestions

recommenders/evaluation/python_evaluation.py Outdated Show resolved Hide resolved
recommenders/evaluation/python_evaluation.py Outdated Show resolved Hide resolved
recommenders/evaluation/python_evaluation.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@miguelgfierro miguelgfierro left a comment

Choose a reason for hiding this comment

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

Looks great!

@miguelgfierro miguelgfierro merged commit cb2299f into recommenders-team:staging Oct 18, 2022
@AdityaSoni19031997
Copy link
Contributor Author

Yay! Its merged! Any other issue that's on priority?
Thanks!

@miguelgfierro
Copy link
Collaborator

hey @AdityaSoni19031997, it's great that you are contributing. Some ideas for you, from easier to harder:

Apart from this, feel free to select an issue on the repo and work on this. The only thing I would advice is that if you select one issue, let me know first to make sure it is not something that is already fixed

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.

None yet

2 participants