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

dash-svm-2022 #minor #705

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

dash-svm-2022 #minor #705

wants to merge 31 commits into from

Conversation

IcToxi
Copy link

@IcToxi IcToxi commented Apr 6, 2022

Hi there,
I made an app and I think you may consider merging it into the project.
Thx

I don't quite understand what this means.

I've summarized the changes requested in the appropriate Streambed issue and confirm that they have been applied.

This is a 2022 replica of dash-svm. I used dash's recent new components and new features, and I've added new page elements.

And I also want to add grids and axes to the canvas, as well as a one-click clear button. But dash-canvas is a totally new component and I can't find where to start. I'm trying to write some client side callbacks to implement these, but not much progress. It would be nice if someone could help enhance this app. Then I can also learn something new from it.

App pull request

  • This is a new app
  • I am improving an existing app (redesigns/code "makeovers")

About

Workflow

  • I have created a branch in the appropriate monorepo, and the
    elements necessary for successful deployment are in place.
  • If the app is a redesigned and/or restyled version of an
    existing gallery app, I've summarized the changes requested in the
    appropriate Streambed issue and confirm that they have been applied.
  • If the app is on the Dash Gallery portal, I have added a link to
    the GitHub repository for the source code in the portal description.
  • If the app is a reimplementation of a Python gallery app for the
    DashR gallery, the app in this PR mimics, as closely as possible,
    the style and functionality of the existing app.=
  • I have removed all Google Analytics code from the app's
    assets/ folder.

The pre-review review

I have addressed all of the following questions:

  • Does everything in my code serve some purpose? (I have removed
    any dead and/or irrelevant code.)
  • Does everything in my code have a clear purpose? (My code is
    readable and, where it isn't, it has been commented appropriately.)]
  • Am I reinventing the wheel? (I have used appropriate packages to
    lessen the volume of code that needs to be maintained.)

Post PR (at merge time)

  • When you are merging, make sure to write one of the following tags in the commit message (or it will default to patch):
    • #patch - An app has been updated or fixed
    • #minor - A new app has been added, or an app has been significantly reworked
    • #major - Breaking changes, make sure to discuss with dash-core before using this tag

Hi there,
I made an app and I think you may consider merging it into the project.
Thx
@IcToxi
Copy link
Author

IcToxi commented Apr 6, 2022

Hi, I would also like to know is there any way to prevent the users from drawing lines in the canvas.

@alexcjohnson
Copy link
Collaborator

@IcToxi this looks great! Thanks so much for your contribution, in fact rather than a new app I'd suggest we use your app to replace dash-svm.

One thing to note though, we've more or less stopped recommending dash_canvas since most thing you can do there can also be accomplished with dcc.Graph - and dcc.Graph can do so much more. Per the dash canvas docs page:

Note: dash-canvas is a legacy package, the recommended way to annotate images is to use the drawing tools of plotly figures.

That said I'm not sure that the techniques described in https://dash.plotly.com/annotations are exactly what you'd want for this purpose... the super-cool point drawing feature you added would really be best handled not by drawing shapes, but by generic click events that cause a data point to be added to a trace. That in turn would be best done with "click anywhere" which is a feature we've been trying for some time to add to plotly.js but the PR to add it has stalled and needs some more work.

So maybe best to leave dash_canvas here, and we can update to dcc.Graph once we manage to finish up click anywhere. There might be a way to pretend we have this feature, by for example putting a heatmap or image on the graph and grabbing click events from the heatmap... if you're feeling really adventurous you could try that.

To your question about streambed - don't worry about it, that's a private Plotly repo, those instructions were meant for Plotly employees and don't apply to you. We're just thrilled that you're contributing here!

cc @danton267 who's starting a project to update many of the apps in this repo - keep an eye on this PR, and leave dash-svm off your list until this PR is merged 😄

@IcToxi
Copy link
Author

IcToxi commented Apr 7, 2022

Hi @alexcjohnson ,
Really appreciate the reply. I'm sorry I missed such a note. I was also thinking about whether canvas could do some interaction with other graphs, and then actually your team was already doing it. Awesome!

So, in fact, that annotation component can not only be used for photos, but actually any chart is within the possible range. Really looking forward to that click anywhere feature. I'm so excited to hear about these.

Thank you very much for taking the time to comment.
BR

@gvwilson gvwilson added fix fixes something broken P3 not needed for current cycle labels Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fixes something broken P3 not needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants