datamol-io / molfeat

molfeat - the hub for all your molecular featurizers
https://molfeat.datamol.io
Apache License 2.0
169 stars 16 forks source link

fix doc site and add etl scripts #54

Closed maclandrol closed 1 year ago

maclandrol commented 1 year ago

Checklist:


Doc site should now look like this: image

maclandrol commented 1 year ago

To do before 0.8.9 ensure no blindspot on cuda based inference

cwognum commented 1 year ago

Using this, we can hide the left-side menu on all pages that have no subpages, but I don't like that the content width then is not consistent. Happy to discuss alternatives!

---
hide:
  - navigation
  - toc
---

# Document title
...
maclandrol commented 1 year ago

Using this, we can hide the left-side menu on all pages that have no subpages, but I don't like that the content width then is not consistent. Happy to discuss alternatives!

---
hide:
  - navigation
  - toc
---

# Document title
...

I don't think we should hide the TOC. Without the TOC, single-page navigation is awful, and you don't have id anchors to jump across the header sections in the API or tutorial page. Honestly, I don't like neither the tab only navigation, nor the inconsistency if you decide to hide the single-page section only. Thus the current solution:

maclandrol commented 1 year ago

Thank you!

  1. I don't like the redundancy from having both tabs and the left-side menu containing the same information. We simplified the left-side menu on purpose after all by introducing the tabs. I would either hide the left-side menu if there's only a single item or remove the left-side menu altogether and add "subtabs" to the each tab that appear on hover.

See answer above. Having subnav is not impossible, but I fear this is a bit more than trying to retheme using the jinja templates.

  1. I like that you've included the ETL notebooks for transparency sake. Could you add a simple README documenting what these notebooks are for? Currently I don't even know what ETL stands for.

ETL means "Extract, transform, load] and commonly refer to the set of set of instructions for process or data integration. Here it corresponds to the process through which we add new models. I will add a note in the developer guideline, but this is intended for developers primarily, not for end users.

cwognum commented 1 year ago

I don't think we should hide the TOC.

I agree! Sorry for the confusion, copied that over from the example but only meant to show how to hide the left-side menu ("navigation"). Don't like the inconsistent content width either though.

For me the main reason to switch to tabs is to reduce the "complexity" (number of items) in the left-side menu. In @DomInvivo his words in the initial proposal: "The old docs API is too messy with too many things on the left. (xref: https://github.com/datamol-io/datamol/pull/184)"

I would argue that now we're instead adding complexity by having redundant tabs. I do understand the benefits you list, but in that case why not just disable the tabs again and stick with what we had before? In the end it's a minor change and I don't have a perfect solution to propose either, so feel free to do as you prefer and merge once ready.

ETL means "Extract, transform, load] and commonly refer to the set of set of instructions for process or data integration. Here it corresponds to the process through which we add new models. I will add a note in the developer guideline, but this is intended for developers primarily, not for end users.

Learned something new today! Thank you! All clear! :pray:

maclandrol commented 1 year ago

Added the notes on ETL for developers.

Note that although using different technologies, that the current version unintentionally reproduced what is used in the Volkamer lab CADD tutorial website (https://projects.volkamerlab.org/teachopencadd/).

If I can't get

add "subtabs" to the each tab that appear on hover.

to work today, I would likely keep the current version, because Dom's version still has the right panel anyway, so my solution just remove the ugly empty section with repeated page title for single-page section.

Removing the right section all together without a way to get an hyperlink to pages in those sections seems worse for me. Happy to try implementing any alternative ideas either you or @DomInvivo might have.

cwognum commented 1 year ago

because Dom's version still has the right panel anyway, so my solution just remove the ugly empty section with repeated page title for single-page section.

Just for clarity: No one wants to remove the right-side panel (i.e. "table of contents").

A final proposal could be to integrate the Table of Contents (right-side) and Navigation (left-side) menus with the Navigation Integration. I would look like this:

image

One orthogonal comment: Going through the docs today, I saw some other cool features, such as the Back to Top button.

maclandrol commented 1 year ago

Just for clarity: No one wants to remove the right-side panel (i.e. "table of contents").

Sorry, I mean the left-side panel (not the TOC in my comment above).

I like the idea of merging them that you are proposing, and some easier things to do may be to reorganize the docs (For example, license as a single tab kinda does not make sense). I will come up with an iteration of your last idea, in next commit a bit later today.

maclandrol commented 1 year ago

Fix #55 and docs look better with grouping, pages together.