Skip to content

refactor: change r precision calculation and documentation#621

Merged
JoanFM merged 2 commits intomainfrom
refactor-evaluation-metrics
Oct 17, 2022
Merged

refactor: change r precision calculation and documentation#621
JoanFM merged 2 commits intomainfrom
refactor-evaluation-metrics

Conversation

@guenthermi
Copy link
Copy Markdown
Contributor

@guenthermi guenthermi commented Oct 13, 2022

Goals:

  • correct the implementation of R-Precision

  • add hints in the documentation about potentially incorrect evaluation scores

  • add tests for all metric functions

  • add links to developer reference of metric functions in the documentation

  • check and update documentation, if required. See guide

@guenthermi guenthermi linked an issue Oct 13, 2022 that may be closed by this pull request
@guenthermi guenthermi marked this pull request as ready for review October 13, 2022 12:53
@guenthermi guenthermi closed this Oct 13, 2022
@guenthermi guenthermi reopened this Oct 13, 2022
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 13, 2022

Codecov Report

Merging #621 (84fda8f) into main (944b91b) will increase coverage by 1.67%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #621      +/-   ##
==========================================
+ Coverage   84.91%   86.58%   +1.67%     
==========================================
  Files         133      133              
  Lines        6715     6717       +2     
==========================================
+ Hits         5702     5816     +114     
+ Misses       1013      901     -112     
Flag Coverage Δ
docarray 86.58% <100.00%> (+1.67%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
docarray/math/evaluation.py 94.82% <100.00%> (+12.68%) ⬆️
docarray/array/mixins/getattr.py 81.81% <0.00%> (+3.03%) ⬆️
docarray/array/mixins/traverse.py 95.04% <0.00%> (+4.13%) ⬆️
docarray/array/mixins/io/csv.py 87.50% <0.00%> (+17.50%) ⬆️
docarray/array/mixins/plot.py 67.96% <0.00%> (+39.82%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@guenthermi
Copy link
Copy Markdown
Contributor Author

guenthermi commented Oct 13, 2022

tests for the changes will be added in #617

@@ -12,20 +12,25 @@ def _check_k(k):


def r_precision(binary_relevance: List[int], **kwargs) -> float:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there needs to be some tests testing the new behavior

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes I agree. Currently there are no tests for the metric functions. I have created a test which tests the behavior together with the evaluate function functions in #617 (which has to be adopted -after one of those Pars is merged), but I can also create tests which directly test the functions docarray.math here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add unit tests yes

Copy link
Copy Markdown

@bwanglzu bwanglzu left a comment

Choose a reason for hiding this comment

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

add tests otherwise looks good

@guenthermi guenthermi force-pushed the refactor-evaluation-metrics branch from a2063fe to 84fda8f Compare October 17, 2022 07:08
@github-actions
Copy link
Copy Markdown

📝 Docs are deployed on https://ft-refactor-evaluation-metrics--jina-docs.netlify.app 🎉

@JoanFM JoanFM merged commit a2e0fe6 into main Oct 17, 2022
@JoanFM JoanFM deleted the refactor-evaluation-metrics branch October 17, 2022 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Minor issues in math.evaluation

3 participants