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

add more docs to matrix filter aka heatmap in error analysis #1489

Merged
merged 1 commit into from
Jun 16, 2022

Conversation

imatiach-msft
Copy link
Contributor

@imatiach-msft imatiach-msft commented Jun 13, 2022

Description

Add more docs to matrix filter aka heatmap in error analysis. It looks like now all methods in erroranalysis package, both public and private, have docstrings.

Checklist

  • I have added screenshots above for all UI changes.
  • I have added e2e tests for all UI changes.
  • Documentation was updated if it was needed.

@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2022

Codecov Report

Merging #1489 (cc5f4aa) into main (addf2a0) will not change coverage.
The diff coverage is 80.00%.

@@           Coverage Diff           @@
##             main    #1489   +/-   ##
=======================================
  Coverage   87.56%   87.56%           
=======================================
  Files         108      108           
  Lines        5081     5081           
=======================================
  Hits         4449     4449           
  Misses        632      632           
Flag Coverage Δ
unittests 87.56% <80.00%> (ø)

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

Impacted Files Coverage Δ
erroranalysis/erroranalysis/_internal/metrics.py 86.20% <ø> (ø)
...is/erroranalysis/_internal/surrogate_error_tree.py 85.80% <ø> (ø)
...ranalysis/erroranalysis/analyzer/error_analyzer.py 92.89% <ø> (ø)
...tabalanceanalysis/distribution_balance_measures.py 100.00% <ø> (ø)
...ranalysis/erroranalysis/_internal/matrix_filter.py 95.85% <80.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update addf2a0...cc5f4aa. Read the comment docs.

@romanlutz
Copy link
Contributor

Is there a way to enforce that it stays this way, i.e., failing the linter or something like that if a new method is added without docstring?

@imatiach-msft
Copy link
Contributor Author

@romanlutz yes, when you run sphinx you can make it fail on missing docs. I'm not sure about using a linter for docs though.

@imatiach-msft
Copy link
Contributor Author

I think if you set warnings as errors in sphinx it will fail on missing method docs:
https://stackoverflow.com/questions/61414199/see-all-warnings-as-errors-when-building-sphinx-docs-on-readthedocs-org

Copy link
Contributor

@romanlutz romanlutz left a comment

Choose a reason for hiding this comment

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

I got quite nitpicky here, so apologies for that, but feel free to ignore as you see fit.

@imatiach-msft
Copy link
Contributor Author

I do see that we have installed:
flake8-docstrings==1.5.0
https://github.com/microsoft/responsible-ai-toolbox/blob/main/requirements-linting.txt#L8
but it doesn't fail on missing docstrings

@imatiach-msft imatiach-msft force-pushed the ilmat/fix-matrix-docs branch from 45e5964 to 530e582 Compare June 16, 2022 14:56
@imatiach-msft imatiach-msft force-pushed the ilmat/fix-matrix-docs branch from 530e582 to cc5f4aa Compare June 16, 2022 15:40
1 similar comment
@imatiach-msft imatiach-msft merged commit 785422e into main Jun 16, 2022
@imatiach-msft imatiach-msft deleted the ilmat/fix-matrix-docs branch June 16, 2022 17:24
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