search

evidentlyai / evidently

Evidently is ​​an open-source ML and LLM observability framework. Evaluate, test, and monitor any AI-powered system or data pipeline. From tabular data to Gen AI. 100+ metrics.
https://www.evidentlyai.com/evidently-oss
Apache License 2.0
5.44k stars 602 forks source link

Feat/add contains link descriptor #1333

Closed Sifr-un closed 1 month ago

Sifr-un commented 1 month ago

Implementing "contains link" descriptor. Resolves #1313

emeli-dral commented 1 month ago

Hi @Rayryu, Apologies for the delay in reviewing your PR. Thank you for your hard work—the implementation looks great! Could you please add the new descriptor to the list of descriptors under the TextEvals heading on the documentation page? Once that’s done, we’ll be ready to proceed with the merge.

Thanks again!

Sifr-un commented 1 month ago

Hi @Rayryu, Apologies for the delay in reviewing your PR. Thank you for your hard work—the implementation looks great! Could you please add the new descriptor to the list of descriptors under the TextEvals heading on the documentation page? Once that’s done, we’ll be ready to proceed with the merge.

Thanks again!

No worries @emeli-dral ! I've added the descriptor under the TextEvals 👌

emeli-dral commented 1 month ago

Hi @Rayryu,

Apologies for any confusion regarding the documentation. It looks like you've made changes to the TextEvals preset itself, which is a bit different. If we keep that change, this descriptor will be included in the preset by default, which isn’t quite what we're aiming for.

I was referring to the documentation only! The document is located here: all-metrics.md#text-evals.

Could I kindly ask you to revert the changes to the TextEvals preset itself and just add a line to the documentation?

Feel free to use the other descriptors as a reference! :)

Sifr-un commented 1 month ago

Apologies for any confusion regarding the documentation. It looks like you've made changes to the TextEvals preset itself, which is a bit different. If we keep that change, this descriptor will be included in the preset by default, which isn’t quite what we're aiming for.

I was referring to the documentation only! The document is located here: all-metrics.md#text-evals.

Could I kindly ask you to revert the changes to the TextEvals preset itself and just add a line to the documentation?

Feel free to use the other descriptors as a reference! :)

Thanks for the clarification! I see what happened now. I'll revert the changes to the TextEvals preset and update the documentation as you suggested. Appreciate your patience and guidance on this!

emeli-dral commented 1 month ago

Hi @Sifr-un ,

Thank you so much for your hard work on this PR! You've done an excellent job, and your efforts are really appreciated. The implementation looks great, and I’m happy to proceed with the merge.

Looking forward to seeing more of your contributions in the future!