🔧 MAINT: Make a more specific selector for no-border

[akhmerov]

closes #287

The original problem was that wildcard CSS leaks, and it's hard to override. Unfortunately, it isn't completely clear to me what the wildcard was supposed to catch in the first place (the description in the source was vague and #287 lacked this info). To figure it out I removed the wildcard and compared the outputs, taking the jupyterbook repo. The proposed change seems to fix the problem with preformatted outputs.

Without wildcard:

With the proposed change:

Identical to the original, as far as I can tell. If I overlooked other problems, do let me know.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

[choldgraf]

Thanks for looking into this - I think that the rest case for this would be using a code cell that generates output with it's own CSS, and confirming that it is not overridden. Did you confirm that as well?

Also I've just enabled precommit ci on this repo so it should fix the jobs when you push another commit

[akhmerov]

Thanks for looking into this - I think that the rest case for this would be using a code cell that generates output with it's own CSS, and confirming that it is not overridden. Did you confirm that as well?

I did. Don't have a screenshot at hand (my book is rebuilding rn and it takes some time), but I am using holoviews, and its UI now renders correctly.

Also I've just enabled precommit ci on this repo so it should fix the jobs when you push another commit

The errors seem to be in Python, which I didn't touch. Plus precommit seems to pass; are you sure that's the problem?

[akhmerov]

I pushed another commit for a good measure.

[akhmerov]

Yup, pretty sure the problems aren't mine.

[choldgraf]

ah yeah you're right - it looks like the tests are failing on master as well because of a change in how the error messages are in the notebooks. will have to figure that out first, sorry about the noise.

[chrisjsewell]

The errors seem to be in Python, which I didn't touch.

Yeh I guess its related to executablebooks/jupyter-cache#70; a package (I'm not sure what one) dropped ipykernel as a direct dependency, which in-turn allowed a newer version of ipykernel to be installed.

We should probably update the tests to handle this new error message format, and also maybe pin the ipykernel version in the testing extra

[chrisjsewell]

The ipykernel now being installed is ipykernel-6.0.3, whereas previously it was ipykernel-5.5.5, so the easiest thing would be to just add ipykernel~=5.5 to the testing extra

[akhmerov]

OK, I'll rebase as soon as #347 is merged.

[codecov]

Codecov Report

Merging #344 (1739c7e) into master (27e9aee) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #344   +/-   ##
=======================================
  Coverage   87.43%   87.43%           
=======================================
  Files          12       12           
  Lines        1337     1337           
=======================================
  Hits         1169     1169           
  Misses        168      168           

Flag	Coverage Δ
pytests	87.43% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27e9aee...1739c7e. Read the comment docs.

[choldgraf]

ok cool - master is fixed and merged into here...is this one ready to go @akhmerov ?

[akhmerov]

To the best of my understanding, yes.

As I wrote initially, I can't be fully certain what the wildcard was supposed to catch. The jupyterbook repo itself looks all good, so at least most things should work well.

[choldgraf]

my intuition is that it's better to have a more restrictive policy that breaks in ways that are easier to debug, rather than a less-restrictive policy that breaks in ways that are hard to debug, so I'd be +1 on just merging this I think

[akhmerov]

I completely agree, let's merge! Just don't want to oversell the content 🙃

[welcome]

Congrats on your first merged pull request in this project! 🎉


Thank you for contributing, we are very proud of you! ❤️