-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 automatically-generated sidebar for documentation #3652
Conversation
@rcurtin is this ready for review? |
@shrit yeah, please go ahead 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these algorithms are not part of the link that you have shared. I do not know why they are not appearing in the sidebar, is this intended?
Also, it took me some time to understand that the triangle on the right side of the classification word is clickable. To be honest I was not able to click on it the first time.
In addition, from a user experience perspective, the words on the sidebar are really small, I know we are still young, but if you have a screen with a high resolution, it is annoying and requires a lot of focus to read with it written.
In my opinion, the sidebar should occupy the entire length of the screen and when you go down it should also follow.
Final note, when clicking on any method inside the classification section, the sidebar reset to its default status. It would be nice for it to show where are we right now, to facilitate the navigation between pages.
These are all my comments, otherwise, this is starting to look really good right now and I really like the first step page, with all the complicated steps and commands, it make the entire library feels easy to work with.
| `datasetInfo` | [`data::DatasetInfo`](../load_save.md#loading-categorical-data) | Dataset information, specifying type information for each dimension. | _(N/A)_ | | ||
| `labels` | [`arma::Row<size_t>`](../matrices.md) | Training labels, [between `0` and `numClasses - 1`](../load_save.md#normalizing-labels) (inclusive). Should have length `data.n_cols`. | _(N/A)_ | | ||
| `dimensionality` | `size_t` | When using on numeric-only data, this specifies the number of dimensions in the data. | _(N/A)_ | | ||
| `numClasses` | `size_t` | Number of classes in the dataset. | _(N/A)_ | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not see the hoeffding tree anywhere in the list, not on the bar or in the main page. Any reason for this disappearance?
|
||
* [mlpack regression techniques](#mlpack_regression_techniques) <!-- TODO: fix link! --> | ||
* [`LinearRegression`](linear_regression.md) <!-- TODO: fix link --> | ||
* [`LARS`](lars.md) <!-- TODO: fix link --> | ||
* [mlpack regression techniques](../../index.md#regression-algorithms) | ||
* [`LinearRegression`](linear_regression.md) | ||
* [`LARS`](lars.md) | ||
* [Bayesian linear regression on Wikipedia](https://en.wikipedia.org/wiki/Bayesian_linear_regression) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bayesian linear regression is not anywhere on the list
@@ -28,7 +26,7 @@ information...](#fully-custom-behavior)) | |||
arma::mat dataset(10, 1000, arma::fill::randu); // 1000 points. | |||
arma::Row<size_t> labels = | |||
arma::randi<arma::Row<size_t>>(1000, arma::distr_param(0, 4)); | |||
arma::mat testDataset(10, 500, arma::fill::randu); // 500 test points. | |||
arma::mat testData(10, 500, arma::fill::randu); // 500 test points. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RF is not anywhere as well
Nope, not intended! I just overlooked them. I added If you refresh now, those are in the sidebar.
This one is a little tricky. I used the HTML5 One easy fix I have is to change the character
If we expand all the sections by default (or don't allow them to collapse), you can end up with a sidebar that's longer than the length of the page. How does the user navigate the sidebar then? They would need a scrollbar specifically for the sidebar... I'd really like to avoid that, then it feels like the page has frames. My thinking was that with collapsible sections, a user would just collapse things if the sidebar was too long for their browser. For an example of a page where the sidebar will get very long, try expanding all the sections on this one: https://www.ratml.org/misc/mlpack-markdown-doc/user/core.html More and more will get added to there as time goes on, and that sidebar will get really long.
I thought so too but I don't know an easy way to do this. When you click any link on the sidebar, it sends you to a totally new page, and so I would need to somehow encode the sidebar state to be passed to the new page. I think it is possible but I don't know how to do it easily. Maybe we can save the idea for later, or maybe someone else has an idea?
I used My main concern was that if the font size is the same as the main page, it gets a bit harder visually to differentiate between the sidebar and the main content. |
I refreshed the page. However I do not see them on the main page, the new methods are in the classification but not in the main page. It is really small, I have added a full-screen capture to give you insight I would not mind it having the same size, there is a lot of space between the sidebar and the main page in the middle. is there any way to make the font adjust automatically in the sidebar so the size of the sidebar would not need to be navigated using a scrollbar and would be always on the same page even if everything is open? Another solution would be to have a dynamic list. Once you open one the other closes automatically, but I think you need JS for this |
[+] and [-] could be better, maybe you can give it a try locally and see if it is easier to use ? |
@shrit can you tell me what you think of the changes I made? http://ratml.org/misc/mlpack-markdown-doc/ I upped the font size in the sidebar a little bit, allowed it to be wider when possible, and changed to |
@rcurtin looks better, Is there any reason why all the methods are not appearing here: |
Ahh, nice catch, I didn't realize the index.md file was missing some algorithms too. Fixed now in ac114f2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second approval provided automatically after 24 hours. 👍
I found that the automatically generated documentation was a little bit hard to browse, especially because some pages got really long. I thought, it would be nice to add a sidebar for easier navigation throughout the whole documentation! Here it is in action, when looking at
core.html
:You can browse a fully-built version of this documentation at https://www.ratml.org/misc/mlpack-markdown-doc/ .
The subsections are collapsible using
<details>
blocks. Unfortunately, that means that the constant part of the sidebar (the "Docs Index" section) needs to be maintained in HTML and not Markdown; this file is added asdoc/sidebar.html
.Then, the second part is custom-generated based on the h1/h2/h3 blocks in each individual page, according to these rules:
user/methods/*.md
, thenh2
s are used for the title andh3
s are used for the sections on the sidebaruser/core.md
, thenh1
is used for the title,h2
s are used for the outer list, andh3
s are used for the collapsible details sublistsh1
is used for the title andh2
s are used for the outer listFor longer-term maintainability, my imagination here is that the scripts that generate the sidebar shouldn't need to be updated often (if at all), and whenever a new method is added or
index.md
is modified, the sidebar will need to be modified too. Therefore, I've made a note of that at the top ofindex.md
.This PR can't really be reviewed correctly until #3639 is merged, so it's not worth looking at the diff until then.