MideTechnology / endaq-python

A comprehensive, user-centric Python API for working with enDAQ data and devices
MIT License
25 stars 12 forks source link

refactored markdown text to fix formatting issues in webinar notebooks #186

Closed CrepeGoat closed 2 years ago

CrepeGoat commented 2 years ago

small formatting fixes for webinar notebooks. This is hard to see from the file diffs, but this includes:

codecov-commenter commented 2 years ago

Codecov Report

Merging #186 (96e0b2f) into development (0f21fd4) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development     #186   +/-   ##
============================================
  Coverage        61.74%   61.74%           
============================================
  Files               34       34           
  Lines             2614     2614           
============================================
  Hits              1614     1614           
  Misses            1000     1000           

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 0f21fd4...96e0b2f. Read the comment docs.

CrepeGoat commented 2 years ago

This looks pretty good, but there's something preventing the python code blocks from doing syntax formatting.

weird. can you give an example that I can look into more?

CFlaniganMide commented 2 years ago

weird. can you give an example that I can look into more?

https://docs.endaq.com/en/feature-refactor-ipynbs/webinars/Webinar_Introduction_NumPy_and_Pandas.html#Manually-or-from-Lists

The build here has a lot of errors that follow the format /home/docs/checkouts/readthedocs.org/user_builds/mide-technology-endaq-python/checkouts/feature-refactor-ipynbs/docs/webinars/Webinar_Introduction_NumPy_and_Pandas.ipynb:: WARNING: Pygments lexer name 'ipython3' is not known

CrepeGoat commented 2 years ago

interesting... imo our tests should catch this before we do, but it looks like they're happily passing with these errors being emitted. I'll add an issue for the tests, and keep working at removing them from this branch (I think I'm now reproducing them locally). Thanks Connor 👍

CrepeGoat commented 2 years ago

Okay, I think I figured it out. Looks like in order to parse the code cells correctly in the docs, nbsphinx needs ipython installed as indicated in the old nbsphinx docs (but not the new docs 🤷‍♂️ ). So I added that in 7fa4cd3dfe399d654611a02b9664a891d0ecc4f0.

On another note, it looks like the pdf building routine was also adding thousands of lines of errors to the logs, mostly due to unicode issues? (e.g., this build.) So just to keep that leaner I also removed the PDF creation routine in 96e0b2fb4f9d3c0edaa1350492b27e07e05f56cc.

But with those two changes it should be good! there are still 4 warnings, but 4 is far fewer than how many there used to be, and I'm not exactly sure how best to avoid the remaining ones atm.

CrepeGoat commented 2 years ago

And as a side-note: this repo has configured readthedocs builds to install dependencies based on the concrete dependencies listed in ./docs/requirements.txt. Right now that file specifies Sphinx==4.2.0, but the current version of sphinx is 4.4.0, so at some point the file could stand to get updated with more recent versions of the required dependencies. But I think that's more appropriate to do in another PR.