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.

Sign up for GitHub

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

Jump to bottom

Added font colour selection for dataframe comparison #68

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Added font colour selection for dataframe comparison #68

wants to merge 3 commits into from
+102 −54

Conversation

calum-mcg
Copy link
Contributor

Description

Added an option to provide a color scheme for font when comparing dataframes. Example:

Usage:

colour_scheme = {
   "default":"light_red",
   "matched":"light_blue",
   "underlined":"purple"    
}
assert_df_equality(df1, df2, allow_nan_equality=False, underline_cells=True, color_scheme=colour_scheme)

The default color scheme (if none provided):

default_colour_scheme = {
   "default":"light_red",
   "matched":"light_blue",
   "underlined":"green"    
}

List of available colors in the bcolors class.

In addition, I've added two checks to validate the color scheme dict:

  • Check correct keys are set
  • Validate color provided is available in bcolors class

Fixes #66

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

  • Passes existing testing suite (poetry run pytest tests)

@calum-mcg
Copy link
Contributor Author

@MrPowers - please let me know if this meets the requirements

@calum-mcg
Copy link
Contributor Author

Once this is merged, I'll work on the refactor ticket

@calum-mcg
Copy link
Contributor Author

Just bumping @MrPowers

@pricemg
Copy link

pricemg commented Sep 15, 2023

any movement on this one @MrPowers? This will be a really good addition beyond the original issue as will very useful for accessibility reasons too.

@calum-mcg
Copy link
Contributor Author

@MrPowers

@MrPowers
Copy link
Owner

This is great, thanks for the contribution.

From a user perspective, I think they'll want to set the colors globally and not have to set this manually for every test.

Setting underline_cells=True and then seperately setting "underlined":"purple" in the color_scheme seems off.

We have a few things we're tracking now:

  • matched rows
  • mismatched rows
  • matched cells
  • mismatched cells

The underline_cells option is poorly named. If we're giving the user full configuration options, then they should be able to underline whatever they want (including matched rows or matched cells).

I am going to have to think about this one some more. We should give the user good defaults, a reasonable mechanism to set global overrides, and the flexibility to set test-level settings.

@MrPowers MrPowers mentioned this pull request Oct 1, 2023
Open
@MrPowers
Copy link
Owner

MrPowers commented Oct 1, 2023

Created an issue to discuss this at a higher level: #75

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robertkossendey robertkossendey robertkossendey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Font colors in error messages are bad in some terminals
4 participants
@calum-mcg @pricemg @MrPowers @robertkossendey