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 run links popover to metrics plot #2295
Conversation
Hey @harupy, @smurching and I took a look at what was causing the issue. We've isolated it to the We used the hook from here to log which prop was changed. Do you think you could take a closer look as to why |
@juntai-zheng Thanks for the investigation. The actual cause is when toggling the popover, the state |
@harupy ah sorry, forgot to mention that we had actually removed |
@juntai-zheng Can you share the branch you're working on? |
@harupy Here it is: https://github.com/juntai-zheng/mlflow/tree/popover Just checking, the links created by the popover link me back to the MLflow home page. Is linking to the appropriate run a WIP? |
Yes |
I checked your branch. In JavaScript, > const a = {};
> const b = {};
> const c = a;
> a === b
false
>>> a === c
true The comparison below doesn't work when prevProps[key] !== val So the metrics wasn't changing. |
One way to freeze the plot when toggling the popover is to use |
Hi @harupy, you were correct in the hook being incorrect and We made rough changes to my branch https://github.com/juntai-zheng/mlflow/tree/popover, so you can check it out from there. |
@juntai-zheng Awesome. I'll reflect the change on this branch. |
@harupy the popover looks great! Looks like a couple test are failing because of extra props being passed with our modifications. Do you think you could apply a fix for the tests? |
@juntai-zheng I will fix the tests! @gioa |
@harupy , the changes look merge ready, would you like to resolve the conflicts? |
@Zangr Fixed the conflict! |
@Zangr What do you think about this? |
Agree, I think we can show a short version and show the full id in the tooltip. cc @gioa |
Yes, that makes sense. I'll open a PR. |
@Zangr @smurching |
#2493) * modified README to include Model Registry component; augmented the Model Registry Workflow API with code snippets * [mlflow/R] Update R client to call record logged model (#2430) * Update R client to call record logged model metadata with tracking server. * fix * Ran roxygen to update R docs. * Updated R NAMESPACE to address cran check complaint. * test fix * fix test * fix test * Addressed review comment + removed forgotten debug string from earlier. * [UI] Better error messages for model registry (#2456) * typo: algorhitm->algorithm (#2490) * Update model registry frontend JS with API changes (#2476) * Save metric plot state in URLs for easier link sharing (#2393) Metric plot config is now saved in the URL to simplify sharing plots & collaborating * Update docs/source/model-registry.rst Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst +1 Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst Good idea to have an actor do the task! Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst thanks for catching the subject-verb agreement! Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update README.rst Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Fix bar chart bug introduced in #2393 (#2498) * minor change and incorporated suggestions * minor typo * mlflow gc command (#2265) * Added hard delete run in sql alchemy store. Added delete artifacts in hdfs store. Added hard delete run endpoint and function in client. * Fixed delete in hdfs artifact repo. Fixed hard delete endpoint. * Add support for file store * Artifacts are deleted on client side when using hard-delete run cli * Added support for local_artifact_repo * Fix linter * Added test for fluent api * Fix tests * Added not implemented yet exception for artifact stores other than hdfs and local * Removed hard delete run endpoint and exposition in client.py * Added gc command line * Fix tests * Remove hard delete runs command line * Add method _get_deleted_runs in sql alchemy store * Added test for _get_deleted_runs in sql alchemy store * Fix linter * Fix linter * Added comments to _hard_delete_run and to the gc cli. * Add implementation of get_deleted_runs for file_store * delete_artifacts now take a relative path. get_deleted_runs in sql alchemry store returns a list of string. Changes to comments. * Fix linter * Fix tests * Added end to end test for mlflow gc command line * Fix windows tests * Fix hdfs_artifact_repo tests on windows * Fix test_cli for python 2 * Fix linter * Fix python 2 tests * Fix python 2 tests * Retrigeer tests becaused of failed R tests * Changed urllib import to use six.moves * Add run links popover to metrics plot (#2295) * Add runs link popover * Rename * Trigger travis * Use ref and refactor * Add onRelayout * Remove unused functions and state * Refactor * Add comment * Fix test * Fix prop type * Rename var * Fix prop type * Add test for RunLinksPopover * Add icon * Refactor * Close popover when Escaoe key pressed * use single quotes * Use named export * Do not expose state * Resolve conflict * Fix popover to use props * Remove unnecessary comment * Remove unnecessary comment * Fix test * Rename popover title * Fix style of close button * Add sorting by y * Display the popover on the left * Display metric value * fix * Fix * Fix * Remove runUuid * Fix test * Fix tests * nit * API cleanup (#2502) * API cleanup removing `stage` field from `UpdateModelVersion` proto * Minor docs tweaks for MLflow gc command (#2506) * Added hard delete run in sql alchemy store. Added delete artifacts in hdfs store. Added hard delete run endpoint and function in client. * Fixed delete in hdfs artifact repo. Fixed hard delete endpoint. * Add support for file store * Artifacts are deleted on client side when using hard-delete run cli * Added support for local_artifact_repo * Fix linter * Added test for fluent api * Fix tests * Added not implemented yet exception for artifact stores other than hdfs and local * Removed hard delete run endpoint and exposition in client.py * Added gc command line * Fix tests * Remove hard delete runs command line * Add method _get_deleted_runs in sql alchemy store * Added test for _get_deleted_runs in sql alchemy store * Fix linter * Fix linter * Added comments to _hard_delete_run and to the gc cli. * Add implementation of get_deleted_runs for file_store * delete_artifacts now take a relative path. get_deleted_runs in sql alchemry store returns a list of string. Changes to comments. * Fix linter * Fix tests * Added end to end test for mlflow gc command line * Fix windows tests * Fix hdfs_artifact_repo tests on windows * Fix test_cli for python 2 * Fix linter * Fix python 2 tests * Fix python 2 tests * Retrigeer tests becaused of failed R tests * Add/improve documentation * Fix indent * resolved and incorporate suggestions * Fixing JS reducer on model version delete (#2507) * resolved and addressed suggestions * fixing search test (unrelated to documentation) * fix docs Co-authored-by: tomasatdatabricks <33237569+tomasatdatabricks@users.noreply.github.com> Co-authored-by: Aaron Davidson <aaron@databricks.com> Co-authored-by: Rens <rens@dimmendaal.com> Co-authored-by: Siddharth Murching <smurching@gmail.com> Co-authored-by: Stephanie Bodoff <stephanie.bodoff@databricks.com> Co-authored-by: t-henri <51711739+t-henri@users.noreply.github.com> Co-authored-by: Harutaka Kawamura <hkawamura0130@gmail.com> Co-authored-by: Mani Parkhe <mani@databricks.com> Co-authored-by: dbczumar <39497902+dbczumar@users.noreply.github.com>
* Add runs link popover * Rename * Trigger travis * Use ref and refactor * Add onRelayout * Remove unused functions and state * Refactor * Add comment * Fix test * Fix prop type * Rename var * Fix prop type * Add test for RunLinksPopover * Add icon * Refactor * Close popover when Escaoe key pressed * use single quotes * Use named export * Do not expose state * Resolve conflict * Fix popover to use props * Remove unnecessary comment * Remove unnecessary comment * Fix test * Rename popover title * Fix style of close button * Add sorting by y * Display the popover on the left * Display metric value * fix * Fix * Fix * Remove runUuid * Fix test * Fix tests * nit
mlflow#2493) * modified README to include Model Registry component; augmented the Model Registry Workflow API with code snippets * [mlflow/R] Update R client to call record logged model (mlflow#2430) * Update R client to call record logged model metadata with tracking server. * fix * Ran roxygen to update R docs. * Updated R NAMESPACE to address cran check complaint. * test fix * fix test * fix test * Addressed review comment + removed forgotten debug string from earlier. * [UI] Better error messages for model registry (mlflow#2456) * typo: algorhitm->algorithm (mlflow#2490) * Update model registry frontend JS with API changes (mlflow#2476) * Save metric plot state in URLs for easier link sharing (mlflow#2393) Metric plot config is now saved in the URL to simplify sharing plots & collaborating * Update docs/source/model-registry.rst Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst +1 Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst Good idea to have an actor do the task! Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst thanks for catching the subject-verb agreement! Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update docs/source/model-registry.rst Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Update README.rst Co-Authored-By: Stephanie Bodoff <stephanie.bodoff@databricks.com> * Fix bar chart bug introduced in mlflow#2393 (mlflow#2498) * minor change and incorporated suggestions * minor typo * mlflow gc command (mlflow#2265) * Added hard delete run in sql alchemy store. Added delete artifacts in hdfs store. Added hard delete run endpoint and function in client. * Fixed delete in hdfs artifact repo. Fixed hard delete endpoint. * Add support for file store * Artifacts are deleted on client side when using hard-delete run cli * Added support for local_artifact_repo * Fix linter * Added test for fluent api * Fix tests * Added not implemented yet exception for artifact stores other than hdfs and local * Removed hard delete run endpoint and exposition in client.py * Added gc command line * Fix tests * Remove hard delete runs command line * Add method _get_deleted_runs in sql alchemy store * Added test for _get_deleted_runs in sql alchemy store * Fix linter * Fix linter * Added comments to _hard_delete_run and to the gc cli. * Add implementation of get_deleted_runs for file_store * delete_artifacts now take a relative path. get_deleted_runs in sql alchemry store returns a list of string. Changes to comments. * Fix linter * Fix tests * Added end to end test for mlflow gc command line * Fix windows tests * Fix hdfs_artifact_repo tests on windows * Fix test_cli for python 2 * Fix linter * Fix python 2 tests * Fix python 2 tests * Retrigeer tests becaused of failed R tests * Changed urllib import to use six.moves * Add run links popover to metrics plot (mlflow#2295) * Add runs link popover * Rename * Trigger travis * Use ref and refactor * Add onRelayout * Remove unused functions and state * Refactor * Add comment * Fix test * Fix prop type * Rename var * Fix prop type * Add test for RunLinksPopover * Add icon * Refactor * Close popover when Escaoe key pressed * use single quotes * Use named export * Do not expose state * Resolve conflict * Fix popover to use props * Remove unnecessary comment * Remove unnecessary comment * Fix test * Rename popover title * Fix style of close button * Add sorting by y * Display the popover on the left * Display metric value * fix * Fix * Fix * Remove runUuid * Fix test * Fix tests * nit * API cleanup (mlflow#2502) * API cleanup removing `stage` field from `UpdateModelVersion` proto * Minor docs tweaks for MLflow gc command (mlflow#2506) * Added hard delete run in sql alchemy store. Added delete artifacts in hdfs store. Added hard delete run endpoint and function in client. * Fixed delete in hdfs artifact repo. Fixed hard delete endpoint. * Add support for file store * Artifacts are deleted on client side when using hard-delete run cli * Added support for local_artifact_repo * Fix linter * Added test for fluent api * Fix tests * Added not implemented yet exception for artifact stores other than hdfs and local * Removed hard delete run endpoint and exposition in client.py * Added gc command line * Fix tests * Remove hard delete runs command line * Add method _get_deleted_runs in sql alchemy store * Added test for _get_deleted_runs in sql alchemy store * Fix linter * Fix linter * Added comments to _hard_delete_run and to the gc cli. * Add implementation of get_deleted_runs for file_store * delete_artifacts now take a relative path. get_deleted_runs in sql alchemry store returns a list of string. Changes to comments. * Fix linter * Fix tests * Added end to end test for mlflow gc command line * Fix windows tests * Fix hdfs_artifact_repo tests on windows * Fix test_cli for python 2 * Fix linter * Fix python 2 tests * Fix python 2 tests * Retrigeer tests becaused of failed R tests * Add/improve documentation * Fix indent * resolved and incorporate suggestions * Fixing JS reducer on model version delete (mlflow#2507) * resolved and addressed suggestions * fixing search test (unrelated to documentation) * fix docs Co-authored-by: tomasatdatabricks <33237569+tomasatdatabricks@users.noreply.github.com> Co-authored-by: Aaron Davidson <aaron@databricks.com> Co-authored-by: Rens <rens@dimmendaal.com> Co-authored-by: Siddharth Murching <smurching@gmail.com> Co-authored-by: Stephanie Bodoff <stephanie.bodoff@databricks.com> Co-authored-by: t-henri <51711739+t-henri@users.noreply.github.com> Co-authored-by: Harutaka Kawamura <hkawamura0130@gmail.com> Co-authored-by: Mani Parkhe <mani@databricks.com> Co-authored-by: dbczumar <39497902+dbczumar@users.noreply.github.com>
What changes are proposed in this pull request?
Resolves #2279
How is this patch tested?
(Details)
Release Notes
Is this a user-facing change?
(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)
What component(s) does this PR affect?
How should the PR be classified in the release notes? Choose one:
rn/breaking-change
- The PR will be mentioned in the "Breaking Changes" sectionrn/none
- No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" sectionrn/feature
- A new user-facing feature worth mentioning in the release notesrn/bug-fix
- A user-facing bug fix worth mentioning in the release notesrn/documentation
- A user-facing documentation change worth mentioning in the release notes