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

Renamed module names and made changes in init to reflect the change #244

Merged

Conversation

tinomaxthayil
Copy link
Contributor

  • Changed module names to _contexts, _context_recall, _faithfullness
  • Changed faithfulness module name in answer_correctness.py
  • Added the new names to init.py

@shahules786
Copy link
Member

  • file names for answer_correctness and other metrics also have to change
  • Add unit tests? @jjmachan

@shahules786 shahules786 self-requested a review November 1, 2023 04:31
@jjmachan
Copy link
Member

jjmachan commented Nov 1, 2023

yes check the documentation on how we are importing it and add tests that mimic that.
also not that critic is something that is now imported like ragas.metric.critic do think about if we should change it

@tinomaxthayil
Copy link
Contributor Author

  • Added test cases and made name changes for the rest of the modules.
  • Critique name is not changed but test case is added for the expected import.

Comment on lines 4 to 9
from ragas.metrics._contexts import (
ContextPrecision,
ContextRelevancy,
context_precision,
context_relevancy,
)
Copy link
Member

Choose a reason for hiding this comment

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

@tinomaxthayil and @shahules786 should we split this into 2 files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you meant Class and instance for _contexts?

Copy link
Member

Choose a reason for hiding this comment

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

@jjmachan Not sure what is ideal here, since both are metrics to evaluate contexts, I would suggest to keep as it is.

@tinomaxthayil
Copy link
Contributor Author

@jjmachan and @shahules786. There were _context_precision and _contexts with the same metrics from another push I assume. I split it into two files and made the init changes. The new files are _contexts and _context_relevancy

Copy link
Member

Choose a reason for hiding this comment

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

This should now be _context_precision

@tinomaxthayil tinomaxthayil reopened this Nov 5, 2023
@shahules786 shahules786 merged commit 9b66ff2 into explodinggradients:main Nov 5, 2023
28 checks passed
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