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

fix plotly incongruity (to be polite) #4584

Merged
merged 9 commits into from
Dec 7, 2022
Merged

Conversation

t-bltg
Copy link
Member

@t-bltg t-bltg commented Dec 7, 2022

Maybe fix #4583.

I don't like doing this (exceptions to the already complicated load / requires mechanism).

Can you test that it works @BeastyBlacksmith ?

@BeastyBlacksmith
Copy link
Member

BeastyBlacksmith commented Dec 7, 2022

julia> using Plots; plotly(); plot(rand(10))
[ Info: Precompiling Plots [91a5bcdd-55d7-5caf-9e0b-520d859cae80]
┌ Warning: backend `GR` is not installed.
└ @ Plots ~/.julia/dev/Plots/src/backends.jl:33
[ Info: GR
[ Info: Precompiling PlotlyKaleido [f2990250-8cf9-495f-b13a-cce12b45703c]

This works. I'm a bit irritated by that warning though

@BeastyBlacksmith
Copy link
Member

BeastyBlacksmith commented Dec 7, 2022

we might want to add a simple test with the plotly backend, that creates a html-file

@t-bltg
Copy link
Member Author

t-bltg commented Dec 7, 2022

I'm a bit irritated by that warning though

You mean the @warn "For saving to png with the `Plotly` backend `PlotlyBase` and `PlotlyKaleido` need to be installed." err ?

We can make it a @debug.

@BeastyBlacksmith
Copy link
Member

BeastyBlacksmith commented Dec 7, 2022

I mean

┌ Warning: backend `GR` is not installed.
└ @ Plots ~/.julia/dev/Plots/src/backends.jl:33

@t-bltg
Copy link
Member Author

t-bltg commented Dec 7, 2022

we might want to add a simple test with the ploty backend, that creates a html-file

As I said, I'm not plotly competent at all, so if you have a test idea ...

@t-bltg
Copy link
Member Author

t-bltg commented Dec 7, 2022

Warning: backend GR is not installed.

How can that happen ? GR is a hard depedency of Plots, maybe run instantiate ?

@BeastyBlacksmith
Copy link
Member

Warning: backend GR is not installed.

How can that happen ? GR is a hard depedency of Plots, maybe run instantiate ?

Hmm.. I had this, but it doesn't show up on CI. Must be my system then.

@t-bltg t-bltg marked this pull request as draft December 7, 2022 15:26
@t-bltg t-bltg force-pushed the plotly branch 3 times, most recently from 8625b88 to c09d2b0 Compare December 7, 2022 16:55
@t-bltg t-bltg marked this pull request as ready for review December 7, 2022 17:48
@codecov
Copy link

codecov bot commented Dec 7, 2022

Codecov Report

Base: 90.64% // Head: 90.74% // Increases project coverage by +0.09% 🎉

Coverage data is based on head (cc3832f) compared to base (6e8c0de).
Patch coverage: 81.63% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4584      +/-   ##
==========================================
+ Coverage   90.64%   90.74%   +0.09%     
==========================================
  Files          41       41              
  Lines        8757     8765       +8     
==========================================
+ Hits         7938     7954      +16     
+ Misses        819      811       -8     
Impacted Files Coverage Δ
src/Plots.jl 88.88% <ø> (ø)
src/backends.jl 81.81% <81.25%> (+0.31%) ⬆️
src/init.jl 100.00% <100.00%> (+26.66%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@t-bltg t-bltg merged commit f492402 into JuliaPlots:master Dec 7, 2022
@t-bltg t-bltg deleted the plotly branch December 7, 2022 17:57
@t-bltg t-bltg changed the title fix plotly incongruity fix plotly incongruity (to be polite) Dec 7, 2022
@BeastyBlacksmith
Copy link
Member

I mean

┌ Warning: backend `GR` is not installed.
└ @ Plots ~/.julia/dev/Plots/src/backends.jl:33

FWIW, I've seen that warning popping up in multiple occasions now, also on other peoples machines. It's working afterwards nevertheless, but it is irritating.

Secondly, I happened to notice that you can't use the plotlybase functionality if you haven't PlotlyKaleido installed, which ideally shouldn't be.

@t-bltg
Copy link
Member Author

t-bltg commented Jan 26, 2023

FWIW, I've seen that warning popping up in multiple occasions now, also on other peoples machines. It's working afterwards nevertheless, but it is irritating.

Maybe an environment stack issue. Could help if we had a reproducer.

@t-bltg t-bltg mentioned this pull request Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Plots don't render using Plotly in v1.37 ("_display is not defined for this backend")
2 participants