Skip to content

Conversation

@tehunter
Copy link
Contributor

@tehunter tehunter commented Oct 6, 2022

cc: @attack68

else self.css["blank_value"],
not all(self.hide_index_),
)
]
Copy link
Contributor Author

@tehunter tehunter Oct 6, 2022

Choose a reason for hiding this comment

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

 column_name = [ _element( "th", ( f"{self.css['blank']} {self.css['level']}{r}" if name is None else f"{self.css['columns_name']} {self.css['level']}{r}" ), name if (name is not None and not self.hide_column_names) else self.css["blank_value"], not all(self.hide_index_), ) ]

Is this a breaking change? For column level names, it removes the index_name class and adds in the columns_names class in it's place. If users were purposefully doing styles with .index_name selectors with the intention of applying those to column level names, it will break that behavior.

Copy link
Contributor

@attack68 attack68 Oct 7, 2022

Choose a reason for hiding this comment

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

Well it will break the example in the user guide because that styles class index_name.

However, this should be an easy fix:

index_names = { 'selector': '.index_name, .col_name', 'props': 'font-style: italic; color: darkgrey; font-weight:normal;' } 

As long as this break is documented I think including the additional functionality is worth it in long run.

@mroeschke mroeschke added the Styler conditional formatting using DataFrame.style label Oct 7, 2022
css_class_names = {"row_heading": "row_heading",
"col_heading": "col_heading",
"index_name": "index_name",
"columns_name": "columns_name",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"columns_name": "columns_name",
"col_name": "col_name",
"row_heading": "row_heading",
"col_heading": "col_heading",
"index_name": "index_name",
"columns_name": "columns_name",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"columns_name": "columns_name",
"col_name": "col_name",
not all(self.hide_index_)
and (r, 0) in self.ctx_columns_names
and self.ctx_columns_names[r, 0]
):
Copy link
Contributor

Choose a reason for hiding this comment

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

is this following the same pattern as above?

@attack68
Copy link
Contributor

attack68 commented Oct 8, 2022

I think this PR looks good but there are a few things to check off:

  • editing the styler guide documentation and any other locations for the list of CSS classes that are added in the HTML: col_name is a new one.
  • although we are re-using the apply and applymap methods with a small extension it is rather overkill to suggest that one would actually use a function to style these. It is probably easier just to supply a small list of styles to each name (of which there are probably a small number). In that case a simple documentation entry might help, something like:
styler.apply_index(lambda s: ["color: blue;", "color: red;"], names=True) 

should suffice.

  • all other column CSS class names have col so I suggested sticking with the format.
  • needs a whats new and a version added for the argument, and a note about minor breaking change.
  • the copying mechanics are also changed. should review the docs there to see if it should be added.
  • the tests seem to be for multiindexes. Does it also work just in the regular index case?
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Nov 8, 2022
@tehunter
Copy link
Contributor Author

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

Still working on this

@tehunter
Copy link
Contributor Author

Note to self on current progress:

@mroeschke
Copy link
Member

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.

@mroeschke mroeschke closed this Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stale Styler conditional formatting using DataFrame.style

3 participants