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.

Sign up for GitHub

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

Jump to bottom

Remove FairlearnDashboard and replace it with matplotlib-based plots #766

Merged
merged 13 commits into from
May 19, 2021
Merged

Remove FairlearnDashboard and replace it with matplotlib-based plots #766

merged 13 commits into from
May 19, 2021

Conversation

romanlutz
Copy link
Member

With a slight delay (originally targeted for April) I'm finally removing the FairlearnDashboard since a newer version already exists in raiwidgets. The documentation is updated to instead use the plots @MiroDudik created with a single line directly from the MetricFrame. In the future we want to add more kinds of plots as already mentioned in #758 #666 and #668 . Specifically, the model comparison plots do not yet have a replacement yet.

Note that the "example" added to the examples directory is not shown under "Example notebooks" on the webpage, which is intentional since it's technically not a notebook.

This also makes #561 mostly redundant, which I'll close shortly.
#667 is also directly addressed with this PR as the examples illustrate.

Roman Lutz added 6 commits November 13, 2020 20:18
first set of removal steps
de51fd9 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
Merge branch 'main' of https://github.com/fairlearn/fairlearn into ro…
43966a1 
…lutz/remove_dashboard

Signed-off-by: Roman Lutz <rolutz@microsoft.com>
replace dashboard with matplotlib plots in example & notebook
f5cf6d4 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
remove img directory and image files
7457988 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
Merge branch 'main' of https://github.com/fairlearn/fairlearn into ro…
e7befde 
…lutz/remove_dashboard
update documentation with new plots
6498d1f 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
@romanlutz romanlutz added enhancement New feature or request documentation labels May 3, 2021
@romanlutz romanlutz self-assigned this May 3, 2021
@romanlutz romanlutz mentioned this pull request May 3, 2021
Closed
Roman Lutz added 2 commits May 3, 2021 16:51
remove MANIFEST, adjust CODEOWNERS, gitignore, REAMDE, bug_report.md,…
12d5b3d 
… several pipelines, build scripts, doc pages mentioning widget/dashboard, and a couple of notebooks.

Signed-off-by: Roman Lutz <rolutz@microsoft.com>
lint
02ef892 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
@romanlutz romanlutz added this to In progress in Metrics May 4, 2021
@romanlutz romanlutz added this to In progress in Visualizations May 4, 2021
romanlutz
romanlutz commented May 4, 2021
View reviewed changes
CODEOWNERS.md Show resolved Hide resolved
examples/plot_grid_search_census.py Outdated Show resolved Hide resolved
examples/plot_grid_search_census.py Outdated Show resolved Hide resolved
docs/quickstart.rst Outdated Show resolved Hide resolved
docs/user_guide/assessment.rst Outdated Show resolved Hide resolved
scripts/build_documentation.py Outdated Show resolved Hide resolved
Roman Lutz added 4 commits May 3, 2021 17:53
doctest fix
b8c50d2 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
doctest adjustments
8c79d14 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
fix doctest for plot
49af5e6 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
add doctest skip
ec7ace9 
Signed-off-by: Roman Lutz <rolutz@microsoft.com>
This was referenced May 4, 2021
Merged
Open
@adrinjalali
Copy link
Member

Could we please have two PRs, one to replace the dashboard with other plotting functionality, and then remove the dashboard itself? Much easier to review.

@romanlutz
Copy link
Member Author

Could we please have two PRs, one to replace the dashboard with other plotting functionality, and then remove the dashboard itself? Much easier to review.

Fair! I was thinking about it the other way round, but that wouldn't have work since it leaves massive holes all over the documentation. The way you suggest is preferable! I'll leave this as the removal PR.

@romanlutz romanlutz mentioned this pull request May 5, 2021
Merged
@romanlutz
Copy link
Member Author

Could we please have two PRs, one to replace the dashboard with other plotting functionality, and then remove the dashboard itself? Much easier to review.

Fair! I was thinking about it the other way round, but that wouldn't have work since it leaves massive holes all over the documentation. The way you suggest is preferable! I'll leave this as the removal PR.

I created #770 for this purpose now. Let's leave this one open at least until #770 is complete. Thanks!

@romanlutz romanlutz mentioned this pull request May 7, 2021
Closed
This was referenced May 7, 2021
Merged
Closed
Closed
@michaelamoako michaelamoako mentioned this pull request May 15, 2021
Open
Merge branch 'main' of https://github.com/fairlearn/fairlearn into ro…
f551b57 
…lutz/remove_dashboard

Signed-off-by: Roman Lutz <rolutz@microsoft.com>
hildeweerts
hildeweerts approved these changes May 18, 2021
View reviewed changes
Copy link
Contributor

@hildeweerts hildeweerts left a comment

Choose a reason for hiding this comment

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

LGTM but I would really like @riedgar-ms or @MiroDudik to have look as well, because I'm not that familiar with the widget part of the library :)

@romanlutz romanlutz mentioned this pull request May 18, 2021
Closed
@romanlutz romanlutz linked an issue May 18, 2021 that may be closed by this pull request
Documentation doesn't list charts #608
Closed
@romanlutz romanlutz mentioned this pull request May 18, 2021
Open
@riedgar-ms riedgar-ms merged commit a3d2516 into main May 19, 2021
@riedgar-ms riedgar-ms deleted the rolutz/remove_dashboard branch May 19, 2021 17:54
@romanlutz romanlutz moved this from In progress to Done in Visualizations May 27, 2021
@rensoostenbach rensoostenbach mentioned this pull request Feb 9, 2022
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hildeweerts hildeweerts hildeweerts approved these changes

@riedgar-ms riedgar-ms riedgar-ms approved these changes

@adrinjalali adrinjalali Awaiting requested review from adrinjalali

@MiroDudik MiroDudik Awaiting requested review from MiroDudik

Assignees

@romanlutz romanlutz

Labels
documentation enhancement New feature or request
Projects
No open projects
Metrics
  
In progress
Visualizations
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Documentation doesn't list charts
4 participants
@romanlutz @adrinjalali @hildeweerts @riedgar-ms