upgrade plotly js - Githubissues

fossasia / visdom

A flexible tool for creating, organizing, and sharing visualizations of live, rich data. Supports Torch and Numpy.

Apache License 2.0

10.04k stars 1.13k forks source link

upgrade plotly js #873

Closed shouldsee closed 2 years ago

shouldsee commented 2 years ago

Description

    ## [shouldsee/visdom/package_version]:latest.min.js not pointing to latest.
    ## see https://github.com/plotly/plotly.py/issues/3651

Motivation and Context

old url points to v1.58.5 and was too old for modern plotly functions. no tests performed on this PR yet

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

[ ] 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)
[ ] Code refactor or cleanup (changes to existing code for improved readability or performance)

Checklist:

[ ] I adapted the version number under py/visdom/VERSION according to Semantic Versioning
[ ] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.

JackUrb commented 2 years ago

@da-h do you know where the visual regression test is failing here for? Reads to me as if the screenshots are new (expected <> to be 0) or missing?

da-h commented 2 years ago

Hey there,

just checked the diff-images from the action-artifacts.
It seems there has been a fix in plotly related to the labels for scatter plots. Thus, in my opinion the errors can be safely ignored.

As you can see below, plotly did add some placeholder labels AA in the old version, which has been fixed with the version proposed in this PR.

I have montaged the initial (current master is left) screenshot, the new (the pr in the middle) screenshot and the diff (on the right) screenshot side-by-side. The only differences occur in:

plot_scatter_text_labels_2d
compare_plot_scatter_text_labels_1d
compare_plot_scatter_text_labels_2d

JackUrb commented 2 years ago

Thanks for digging in @da-h, and thanks @shouldsee for making this upgrade!