flatironinstitute / nemos

NEural MOdelS, a statistical modeling framework for neuroscience.
https://nemos.readthedocs.io/
MIT License
82 stars 8 forks source link

renaming files in dev notes #231

Closed BalzaniEdoardo closed 1 month ago

BalzaniEdoardo commented 1 month ago

Reordered the dev notes in a more meaningful way. Corrected the leftover RecurrentGLM class.

codecov-commenter commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.94%. Comparing base (32132b1) to head (5c9adb2). Report is 291 commits behind head on development.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## development #231 +/- ## ================================================ - Coverage 97.30% 75.94% -21.36% ================================================ Files 18 24 +6 Lines 1669 2353 +684 ================================================ + Hits 1624 1787 +163 - Misses 45 566 +521 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sjvenditto commented 1 month ago

Some of the page links were broken with the new page numbers, so I went through and fixed them

BalzaniEdoardo commented 1 month ago

I finally took care of the broken links issue. Now we have a post_build job defined in the .readthedocs.yml that runs html-proofer on the built website. Any internal or external (404) broken links will result in the build to fail;

I am also waiting that mkdocs-gallery merges my PR, then we will be able to switch to a "strict" build for mkdocs. This mode checks only the internal links, but doesn't check the external, so we will need to keep the htmlproofer call.

Anyways, htmlproofer is stricter than mkdocs (doesn't allow links to ".md" for example, while mkdocs does), and checks even the autogenerated links (since it goes through the html); I really like the tool, we should keep rely on it

BalzaniEdoardo commented 1 month ago
  • htmlproofer looks great! It's this tool, right? I've been meaning to add something like this to plenoptic

Yes, that's the tool, it works great!

  • Can you write something in this PR about why you're invoking it the way you are? I'm confused about calling on everything but the reference and then just the reference

The issue was that htmlproofer is quite opinionated about how the html section should be formatted. And for specific paragraphs it requires an href to be specified. However we are not generating the html for the reference, mkdocstrings is doing that, and doesn't format the html that way. (The pages still renders well buy they have the href field missing in some places) This causes the htmlproofer to fail without a flag to alle missing href.

For the rest of our html pages however we can be stricter, this is why I have to calls to htmlproofer.

  • I see you removed the .md from some of the links. That file extension was showing up in the built html? I was pretty sure that .md got converted to html (or the relevant folder), but maybe that's how sphinx does it and I'm confused

This Is again for the htmlproofer. The links would work ok even with the ".MD" extension, but the html proofer thinks that the link with the extension are wrong because those MD files will are converted to HTML as a folder with the same name of the file, containing an index.html, so there is no "site/path_to_page/scriptname.md" but a "site/path_to_page/scriptname/index.html" instead. (so you can set the link to that folder and to pass the htmlproofer check)

  • Can you write a description of what the changes you made to gen_ref_pages.py do?

Will do!

  • I see you changed how the footnote / references work in only one place. Why only in simulation.difference_of_gammas (and not, e.g., basis.RaisedCosineBasisLog (which seems to work fine)?

This one was broken, so I fixed it. Then I tried to have the reference formatted as numpydoc (without the section header tag) but I did not think that it would be rendered badly as docstrings

  • Additionally, I don't like the HTML in the docstring that this change requires -- it makes the docstring much harder to read from ipython / jupyter / IDE, and will probably confuse our users

Good point , I did not think about that

BalzaniEdoardo commented 1 month ago

Changes to gen_ref_pages.py

This PR edited the configuration script for the automatic generation of the documentation page.

After we introduced a hierarchy in the folder structure, i.e. sub-modules likesrc/nemos/fetch, the gen_ref_pages.py script as was originally structured generated broken links as well as did not reflect well the hierarchical structure of the modules.

Edits