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

DOC: Add dashboard question to FAQ and how to save a plot #768 #1021

Merged
merged 11 commits into from
Apr 8, 2022

Conversation

rensoostenbach
Copy link
Contributor

@rensoostenbach rensoostenbach commented Feb 9, 2022

This PR addresses the two TODO's in #768:

  • Create a Q&A section, something along the lines of: "What happened to the FairlearnDashboard?" [answer should point to the new repo and to our plotting functionality through MetricFrame]
  • add a line to the newly created plotting documentation in Remove FairlearnDashboard and replace it with matplotlib-based plots #766 to show how one can save the generated matplotlib plots.

I am unsure if https://fairlearn.org/main/user_guide/assessment.html#fairlearn-dashboard should be completely removed from the assessment page, since the first paragraph of the assessment page still links to it. Perhaps the :ref:'dashboard' should be changed to :ref:'plot'? (intentionally using the wrong quotes here)

docs/faq.rst Outdated Show resolved Hide resolved
docs/faq.rst Outdated Show resolved Hide resolved
docs/faq.rst Outdated Show resolved Hide resolved
docs/user_guide/assessment.rst Outdated Show resolved Hide resolved
Copy link
Member

@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.

Love it! Thanks for this. I added just one small comment.

docs/user_guide/assessment.rst Outdated Show resolved Hide resolved
Copy link
Member

@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.

This looks great to me. Ideally, you could put a screenshot from the generated docs (only the part you changed, of course) to give reviewers an idea of what it looks like after your change is applied 🙂 In the new PR template that's sort of suggested with a "Screenshots" section but your PR preceeds that I think, so no worries.

@romanlutz
Copy link
Member

@rensoostenbach can you post a screenshot of the resulting website?

@rensoostenbach
Copy link
Contributor Author

@rensoostenbach can you post a screenshot of the resulting website?

Yes ofcourse!
user guide change
faq change

@romanlutz
Copy link
Member

Love it! Can one of @fairlearn/fairlearn-maintainers take a look? I approved already so feel free to merge if you are fine with the PR as is.

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

LGTM

@adrinjalali
Copy link
Member

not sure why "Squash and merge" is disabled for me, will wait for the CI to see what happens next.

@adrinjalali adrinjalali merged commit 3335c33 into fairlearn:main Apr 8, 2022
@rensoostenbach rensoostenbach deleted the faq_fairlearn_dashboard branch April 19, 2022 06:52
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

3 participants