facets icon indicating copy to clipboard operation
facets copied to clipboard

Feature/dynamically set sprite sizes

Open contractorwolf opened this issue 7 years ago • 12 comments

modified the approach from suggestions on https://github.com/PAIR-code/facets/pull/118

contractorwolf avatar Apr 07 '18 19:04 contractorwolf

@jimbojw better? worse?

contractorwolf avatar Apr 11 '18 04:04 contractorwolf

@jameswex can I get a ruling on the new PR? I actually had some new ideas for UI changes to the deep dive display that will work better for jupyter notebook as well. If you guys take this PR (or give me some changes you want for it) I can get started on those other updates

contractorwolf avatar Apr 14 '18 19:04 contractorwolf

I believe @jimbojw is taking a look at this PR.

As far as UI changes, our visual designer is looking at making some changes to Facets Dive. Please refrain from any visual changes to Facets Dive at this time. If you have ideas for visual changes to help with uses in notebooks, a simple explanation or mock would be better than a PR so we could discuss it with our visual designer.

jameswex avatar Apr 16 '18 13:04 jameswex

@contractorwolf sorry for the delay, I've been having trouble assessing the PR. The code looks fine, but I've been having trouble running it. It's probably a problem with my setup, but wanted to see it in action before signing off.

jimbojw avatar Apr 16 '18 17:04 jimbojw

@contractorwolf got it working, looks good. Just to confirm, this only affects the demo ipynb, correct?

jimbojw avatar Apr 16 '18 18:04 jimbojw

@contractorwolf you don't have to make any more changes to the ipynb file. The reason I couldn't look at the diff is because it was too large, but that's OK, I was able to run it locally. I just want to confirm that this is the only file you intended to change, right?

jimbojw avatar Apr 17 '18 15:04 jimbojw

@jimbojw yeah, I was only making the change to that one file, I cant seem to figure out why github is having trouble displaying it but I merged my changes in to my master and can show them here https://nbviewer.jupyter.org/github/contractorwolf/facets/blob/master/facets_dive/Dive_demo.ipynb

seems like github has some issues still with the display of notebook files :/

contractorwolf avatar Apr 17 '18 17:04 contractorwolf

my changes were to figure out how to get github to display correctly, but it never did so I went back to my original code from the PR, which is what I showed before for the 119 PR

contractorwolf avatar Apr 17 '18 17:04 contractorwolf

@jimbojw I was trying to get the ipynb file to display correctly in github, it was the output html from that but it still doesnt display unfortunately, you can ignore that it you want

contractorwolf avatar Apr 27 '18 02:04 contractorwolf

@jimbojw I can do another PR without that if you want, just let me know

contractorwolf avatar Apr 27 '18 02:04 contractorwolf

No worries, I'm willing to merge this one, but without the HTML file. So if you can remove that, I'll push it 😎

jimbojw avatar Apr 27 '18 16:04 jimbojw

@jimbojw and @jameswex the new PR without the HTML file: https://github.com/PAIR-code/facets/pull/123

contractorwolf avatar Apr 28 '18 03:04 contractorwolf