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

Add workaround for broken SVGs in nbconvert #575

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

desilinguist
Copy link
Member

Newer versions of nbconvert use a "clean_html" filter on SVG elements that break SVG rendering; so we override that filter here with a custom function that is essentially a noop. For more details, please refer to #571.

To review this properly, please run the following on both your Macs and on a Linux server (EC2 instance will work too):

  • Clone RSMTool and switch to this branch
  • If you are running on your Mac, run CONDA_SUBDIR=osx-64 mamba create -n rsmdev --file requirements.txt python=3.8 to create the conda environment. If on a Linux server, run mamba create -n rsmdev --file requirements.txt python=3.8 instead.
  • Activate the newly created environment and run pip install -e .
  • Run rsmtool tests/data/experiments/lr-subgroups-with-h2/lr_subgroups_with_h2.json foobar.
  • Run open foobar/report/lr_subgroups_with_h2_report.html and make sure that all the figures in the report are visible and as expected.

@codecov
Copy link

codecov bot commented Dec 2, 2022

Codecov Report

Base: 95.74% // Head: 95.75% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (199fdd8) compared to base (7ea1082).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #575   +/-   ##
=======================================
  Coverage   95.74%   95.75%           
=======================================
  Files          29       29           
  Lines        3996     4000    +4     
=======================================
+ Hits         3826     3830    +4     
  Misses        170      170           
Impacted Files Coverage Δ
rsmtool/reporter.py 92.64% <100.00%> (+0.14%) ⬆️

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.

Copy link
Contributor

@Frost45 Frost45 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on my Mac and everything works as expected!

@tamarl08
Copy link
Contributor

tamarl08 commented Dec 2, 2022

@desilinguist I tested on linux. some plots are not shown in the report, only black boxes. otherwise it's ok.
(after the test completed I copied the foobar folder to my mac and opened the report)

@desilinguist
Copy link
Member Author

@tamarl08 you saw block boxes on this branch? That's what this PR is supposed to fix so something didn't work right for you. I had also tested on Linux and didn't see any black boxes. Are you sure you tested this branch and not develop?

@tamarl08
Copy link
Contributor

tamarl08 commented Dec 2, 2022

Of course I tested main 😬

@tamarl08
Copy link
Contributor

tamarl08 commented Dec 2, 2022

looks good on the branch!

@desilinguist desilinguist merged commit a77d3f2 into main Dec 2, 2022
@delete-merged-branch delete-merged-branch bot deleted the 571-fix-broken-svg-rendering branch December 2, 2022 18:36
@damien2012eng
Copy link
Contributor

Tested on EC2. Works as expected!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants