microsoft / archai

Accelerate your Neural Architecture Search (NAS) through fast, reproducible and modular research.
https://microsoft.github.io/archai
MIT License
467 stars 92 forks source link

Adjusting the notebook's HTML table to fit the documentation page #204

Closed luisffranca closed 1 year ago

luisffranca commented 1 year ago

In the documentation the HTML table is being squeezed:

image

[UPDATE]: The CSS documentation was adjusted to avoid it.

Changes

Please delete options that are not relevant.

luisffranca commented 1 year ago

I just checked the latest doc build and this fix will not solve the problem:

image

I'll work in another solution and update the PR.

lovettchris commented 1 year ago

I'll work in another solution and update the PR. How about we just remove the HTML table single cell output, I believe the doc generator will just skip it.

lovettchris commented 1 year ago

Hey I just found this in the algos.ipynb, perhaps this is a better way to go, let the notebook do native rendering of the dataframe?

image

This seems to render fine in this version: https://microsoft.github.io/archai/getting_started/notebooks/discrete_search/algos.html

lovettchris commented 1 year ago

Cool, I tried this and even added a nice filter to limit the number of rows and it looks great, I pushed this change to my notebook in your branch, if you do the same in your notebooks I think this will be good to go:

from IPython.display import display from IPython.core.display import HTML

df = nb_helper.get_search_csv(output_folder) df.drop(columns=['parent', 'parents'], inplace=True) df[df["AmlTrainingValAccuracy"] > 0.98]

luisffranca commented 1 year ago

Cool, I tried this and even added a nice filter to limit the number of rows and it looks great, I pushed this change to my notebook in your branch, if you do the same in your notebooks I think this will be good to go:

from IPython.display import display from IPython.core.display import HTML

df = nb_helper.get_search_csv(output_folder) df.drop(columns=['parent', 'parents'], inplace=True) df[df["AmlTrainingValAccuracy"] > 0.98]

Thanks for the suggestion Chris! However, I still get a table like the following after I pulled your changes and built the doc locally:

image

Even the algos.ipynb has some rendering problems in their header:

image

gugarosa commented 1 year ago

There is a hacky way to fix it via CSS. I tested it locally and seems to be working, at least for the tables that are depicted in the current notebooks (not sure if new tables will break or work accordingly).

Just add this at the end of docs/assets/css/custom.css:

div.output_area.rendered_html.docutils.container > div > table {
    width: auto !important;
}

The selector is one of the most specific ones just to avoid breaking other tables.

luisffranca commented 1 year ago

There is a hacky way to fix it via CSS. I tested it locally and seems to be working, at least for the tables that are depicted in the current notebooks (not sure if new tables will break or work accordingly).

Just add this at the end of docs/assets/css/custom.css:

div.output_area.rendered_html.docutils.container > div > table {
    width: auto !important;
}

The selector is one of the most specific ones just to avoid breaking other tables.

Thanks Gustavo, that worked! I pushed this change to the PR.

gugarosa commented 1 year ago

Sweet! Please feel free to merge

lovettchris commented 1 year ago

Luis, the CSS change also fixed these pages:

and so I think your tutorials could do the same and then we don't need the get_csv_as_stylized_html helper function at all and then your tables wouldn't have that weird teal background and they would look more consistent with the other tables, with the nice alternating row color background UX:

image

luisffranca commented 1 year ago

Luis, the CSS change also fixed these pages:

and so I think your tutorials could do the same and then we don't need the get_csv_as_stylized_html helper function at all and then your tables wouldn't have that weird teal background and they would look more consistent with the other tables, with the nice alternating row color background UX:

image

I kept the get_csv_as_stylized_html in the other notebooks to keep the scrollbar, otherwise the output in the notebook itself (not in the docs) is too large.

As an alternative I can filter the output a little bit to keep a smaller number of rows, and then drop the get_csv_as_stylized_html.

lovettchris commented 1 year ago

when I squeeze the window I still see a horizontal scroll bar, are you are talking about horizontal or vertical scrollbar?

image

luisffranca commented 1 year ago

Vertical scrollbar.

lovettchris commented 1 year ago

Ah, vertical scrollbars in Jupyter notebooks are controlled by their UX, see https://github.com/microsoft/vscode/issues/118117 and https://github.com/microsoft/vscode-jupyter/issues/5629 where they added a setting jupyter.enableScrollingForCellOutputs which I believe you can place in the notebook. It seems the philosophy of vscode jupyter team is that nested scrollbars are not nice and I tend to agree, they would rather one vertical scroll bar for the notebook, so you could also select a subset of the rows so that lots of vertical scrolling is not necessary. Ask yourself, do you expect the user to actually play with that scroll bar? Will they learn something useful by doing that? THen also think about the complexity of mouse wheel scrolling - they are scrolling the notebook until they hit your table then they get stuck scrolling inside the table instead of the notebook, not idea.

luisffranca commented 1 year ago

Got it! Thanks Chris, I didn't know about this phisolophy of the vscode jupyter team. I'll filter the output then and update the notebooks.

luisffranca commented 1 year ago

Done.

lovettchris commented 1 year ago

https://microsoft.github.io/archai/advanced_guide/cloud/azure/notebooks/quickstart/quickstart.html looks great, thanks.