Skip to content
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

Added style to table representation #17

Merged
merged 6 commits into from
Jan 24, 2022
Merged

Conversation

germa89
Copy link
Contributor

@germa89 germa89 commented Jan 21, 2022

Hi there

Since we would like to use Pandas DataFrame to present data in PyMAPDL documentation, I thought we might as well include the style in the Pyansys Theme.

The proposal is shown as this:

image

I'm happy to do any changes you might propose. I just tried the colors align with Ansys color theme.

@germa89 germa89 added the enhancement General improvements to existing features label Jan 21, 2022
@germa89 germa89 self-assigned this Jan 21, 2022
@germa89
Copy link
Contributor Author

germa89 commented Jan 21, 2022

Slightly improved version:

image

Notes

This affects any table in the documentation (see for example tech demo in User guide > Extended examples). It looks super good in my opinion, specially for big tables.
image

The issue is it ALSO affects autosummary (not sure why).

image

This is not a good effect.

Proposal

I wonder if someone more skilled than me can sort out how to create a table style in CSS which is only applied to Dataframes.
I believe by default the style for dataframes is something like Dataframe table or similar (please check this). If it is like that, we should just over write that style in our ansys.css.

I did a bit of research and I didn't find the way to do this. However, if requested I can keep trying.

Copy link
Contributor

@Andy-Grigg Andy-Grigg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are nice! I like the colors and the highlighting.

I'm seeing some slight weirdness with building the docs for an as-yet-unreleased package I'm working on, I'll follow up with you separately on what I'm seeing there.

I am by no means a css expert, but I've added some comments that I think should help limit the changes to affect dataframes only.

@germa89
Copy link
Contributor Author

germa89 commented Jan 21, 2022

Final proposal:

  • The general style (colouring) is common to tables, dataframes and longtable.
    image

  • Tables do not have hovers.
    image

  • Dataframes and longtable do have hovers.
    image

  • autosummary tables have all their rows white and do have hovers (the rest of the format is inherited from table)
    image

@Andy-Grigg
Copy link
Contributor

I have added a few more comments. I like what you've done with the comments and adding in specific sections for different types of tables, it's a bit more verbose but I think it's the right way to go.

I have only tested the dataframe table type, because that's the only one our docs are currently using. I'll have to rely on you or Alex to test out some of the other table types you have added to the CSS.

@akaszynski
Copy link
Contributor

I have only tested the dataframe table type, because that's the only one our docs are currently using. I'll have to rely on you or Alex to test out some of the other table types you have added to the CSS.

I typically use sphinx tables and pandas.DataFrame, so we're going to have to test out the other styles as we go.

I think these tables look excellent. Nice work @germa89 (and by extension, @RomanIlchenko1308 who I think helped inspire this work with VM examples).

@germa89
Copy link
Contributor Author

germa89 commented Jan 24, 2022

I typically use sphinx tables and pandas.DataFrame, so we're going to have to test out the other styles as we go.

In PyMAPDL you can find:

If this theme modification is successful I guess we will get rid of formatted dataframes, in favour of plain dataframes which will inherit the settings (colors, etc) from the theme.

I think disregarding the coloring issue with the dataframe odd rows, we are good to go.

Copy link
Contributor

@Andy-Grigg Andy-Grigg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @germa89, the changes look good! I ran a build of the Granta docs based on this branch, and the dataframe outputs look great. Good to merge.

@germa89 germa89 merged commit 3b65ec3 into main Jan 24, 2022
@germa89 germa89 deleted the feat/dataframe-table-format branch January 24, 2022 15:15
@RomanIlchenko1308
Copy link

RomanIlchenko1308 commented Jan 25, 2022

Hi @germa89 and @Andy-Grigg,

At first, I would like to thank you guys for your work, it looks nice!!!

I am writing this comment because, I have accidentally discovered weird behavior of the highlighting the rows when you select some rows in the data frame on the Non-available Commands it is not an issue, but, it looks weird a bit for me:

that we have a connectivity (highlighting selections) of the first row and the index column with column, as you can see below:

image

But, if you move your mouse down, we have just a partial hover selection of the columns but without indexing column as you can see it below:

image

And if you are going to zoom away and see how it looks like, you can see it below:

image

My question:
Can we make the table in the same style ?

If I am wrong, please write me)
thank you in advance @germa89 and @Andy-Grigg !!!

@Andy-Grigg
Copy link
Contributor

Hi @RomanIlchenko1308!

I think the issue is caused by the merged rows, and I'm not really sure what the best option is to fix it. The first column is taking the color of the row that it starts on. The same thing is happening when it highlights as well.

I don't think there's much we can do to make the first column highlight correctly, because the CSS can only ever highlight a cell all the same color. I think all we can do is not highlight tables with merged cells, but I don't know if that's possible. @germa89, any thoughts?

@germa89
Copy link
Contributor Author

germa89 commented Jan 26, 2022

We could create a class for non-highlighted tables if needed, so we leave it at the writer if he wants that behavior or not.

Other than that, I do not think I can fill the color issue. It is way beyond my expertise. I could try, but the amount of time required vs reward, it is very low.

I would leave it as it is. Or if @RomanIlchenko1308 wants to implement the non-highlighted class, I'm happy to review them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants