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

Feature/dual metrics widget #140

Merged
merged 28 commits into from
May 28, 2024

Conversation

Gooogr
Copy link
Contributor

@Gooogr Gooogr commented May 16, 2024

Description

Add a Plotly scatterplot widget for metrics visualization, enabling support for visualizing data per fold or their average values.
Closes #93

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Optimization

How Has This Been Tested?

Before submitting a PR, please check yourself against the following list. It would save us quite a lot of time.

  • Have you read the contribution guide?
  • Have you updated the relevant docstrings? We're using Numpy format, please double-check yourself
  • Does your change require any new tests?
  • Have you updated the changelog file?

CHANGELOG.md Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
tests/visuals/test_metrics_app.py Outdated Show resolved Hide resolved
@blondered blondered requested a review from feldlime May 17, 2024 09:44
pyproject.toml Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
@Gooogr
Copy link
Contributor Author

Gooogr commented May 19, 2024

  • Update CHANGELOG.md
  • Update poetry files
  • Update code according to the comments above

CHANGELOG.md Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Show resolved Hide resolved
@Gooogr Gooogr marked this pull request as ready for review May 20, 2024 21:45
@Gooogr
Copy link
Contributor Author

Gooogr commented May 20, 2024

Updated the code based on feedback and added docstrings. Remaining tasks:

  • Create a Jupyter notebook example
  • Update the poetry lock file and get last updates from master branch

pyproject.toml Show resolved Hide resolved
@Gooogr Gooogr marked this pull request as draft May 22, 2024 21:46
@Gooogr
Copy link
Contributor Author

Gooogr commented May 22, 2024

Changed the principle of widget design, now it relies on go.WidgetFigure. Also switch from plotly express to plotly graph object scatter chart, because WidgetFigure doesn't support express wrapper.
TODO:

  • Simplify display method
  • Expand MetricsApp documentation
  • Add MetricsApp to compat.py
  • Probably add width and height as optional parameters again.

Currently, support for plotly_kwargs has been removed due to the large amount of customization inside the display method. But the users would be able to customize the chart appearance at the static rendering stage. Add example in jupyter notebook.

@Gooogr Gooogr marked this pull request as ready for review May 23, 2024 17:52
@Gooogr
Copy link
Contributor Author

Gooogr commented May 23, 2024

Update MetricsApp

  • Refactor display method
  • Add jupyter example
  • Add docstring example
  • Update tests

Add optional parameters support from plotly.go.Layout

rectools/visuals/__init__.py Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
rectools/visuals/metrics_app.py Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
rectools/visuals/metrics_app.py Show resolved Hide resolved
rectools/visuals/metrics_app.py Show resolved Hide resolved
rectools/visuals/metrics_app.py Show resolved Hide resolved
rectools/visuals/metrics_app.py Outdated Show resolved Hide resolved
@blondered
Copy link
Collaborator

blondered commented May 24, 2024

Let's add explicit info for new users about the use of MetricsApp in 2_cross_validation.ipynb:

  1. "And let's visualuze metrics" ->
    In RecTools we have interactive MetricsApp for detailed analysis of metrics trade-off between different models.
    visuals extension is required to run the path of code below. You can install it with pip install rectools[visuals]

  2. models_metrics.rename(columns={"model": Columns.Model, "i_split": Columns.Split}) - this we can drop, the names are already the same

  3. app = MetricsApp.construct(pd.DataFrame(cv_results["metrics"])) for one line of code instead of two

  4. Before app screenshot: "If you run this notebook, you will get interactive widgets with active buttons to select metrics and folds. For offline presentation we keep static screenshots of the actual app."

  5. After app screenshot: "If you want to save static image from your app, you can access plotly.graph_objs.Figure and render it as an image. Make sure you have kaleido package to process rendering. You can install it with pip install kaleido==0.2.1 and restart the kernel.

@blondered blondered merged commit 3fe8c31 into MobileTeleSystems:main May 28, 2024
7 checks passed
blondered pushed a commit that referenced this pull request Sep 23, 2024
Added MetricsApp for interactive model metrics comparison
Closes #93
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.

Widgets for simultaneous dual metrics analysis
3 participants