fossasia / visdom

A flexible tool for creating, organizing, and sharing visualizations of live, rich data. Supports Torch and Numpy.
Apache License 2.0
9.99k stars 1.14k forks source link

fixed issue where large image would render in different env #891

Open Aryanamish opened 1 year ago

Aryanamish commented 1 year ago

Updated the backend to send the environment ID with the render information.

Updated the Front End to ignore the incoming window when it is not in the currently selected environment

Motivation and Context

issue #80

Types of changes

Checklist:

JackUrb commented 1 year ago

Hm I'm unsure why the tests are failing here, it may be a good idea to rebase then regen the minified frontend.

da-h commented 1 year ago

Great bugfix!

As a side-note:

Aryanamish commented 1 year ago

I fixed the linting error

da-h commented 1 year ago

The failing of the functional test seems legitimate to me: This PR in its current form does not work for forked environments.

Reason is that the state of each window does not yet adapt to the new eid when forking. (See https://github.com/fossasia/visdom/blob/master/py/visdom/server/handlers/web_handlers.py#L472). I.e. the windows are stored under the forked eid, however the windows (local) state contain the eid the windows belong as well, which has not been adapted, yet.

The Python Linter-Error can be fixed easily.

The visual regression tests: see update below.

Regarding the other failing tests:

  1. ~What I do not understand, yet, is why the visual regression tests are failing. The error says that the visual test-image dimensions do not match, which i cannot confirm from looking shortly into it. I'll look into it later/tomorrow in more detail.~ (For update, see below).
  2. The Javascript Linter can be ignored for now. I have a solution at hand, fixing all these errors, but need just a bit more time to finish the PR conscientiously.

Best, da-h

da-h commented 1 year ago

Update:

I've checked the visual regression test, which is legitimately failing as well:

For instance the end-state for the test compare_plot_line_multiple looks like this: Compare with compare-view screenshots -- Compare screenshot for plot_line_multiple (failed)

but should contain this compare plot: compare_plot_line_multiple