-
-
Notifications
You must be signed in to change notification settings - Fork 78
Enhance savefig() #494
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
Enhance savefig() #494
Conversation
Looks like windows tests fail due to kaleido 0.2 issue not related to PR. |
Thanks for the PR, I'll try to take a look soon. |
Would this adress #503 ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the nice contribution. Sorry for the delay in reviewing.
Make consistent with sglyon/PlotlyBase.jl#111
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #494 +/- ##
===========================================
+ Coverage 28.67% 51.81% +23.13%
===========================================
Files 8 8
Lines 265 276 +11
===========================================
+ Hits 76 143 +67
+ Misses 189 133 -56
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
We will fix the documentation failure (not related to this PR) in a separate PR |
This PR does 2 things:
savefig()
: combine docstring from all 3 versions, streamline keyword argument passing usingkwargs...
plotlyjs
andplotly_version
keyword arguments tosavefig()
to control which plotly.js Kaleido would be using to generate the figures.This is to allow using more recent versions of plotly.js than the one provided by the PlotlyJS.jl (the current one, 2.3.0, is more than a year old).
I have also regenerated the plotly.js artifact using version 2.35.2.
I can provide the file, or the maintainers can run
generate_artifacts.jl
themselves.See also comment for PlotlyBase.jl#106