Skip to content

Added error messages for scatterplot facet #4893

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

Merged
merged 2 commits into from
Jun 15, 2022

Conversation

elebitzero
Copy link
Member

@elebitzero elebitzero commented May 26, 2022

Fixes #4890

Changes proposed in this pull request:

  • Added error messages to prevent users from attempting to do a scatter plot facet without the requisite columns.

Let me know if the wording of the error message is alright:

When column is not numeric:

"Scatterplot requires a numeric column (green values). The column $1 has less than 50% numeric data. If a column has numerical data but is stored as text (black color), then use Column->Edit Cells->Common Transforms->to Number to convert."

image

When column is numeric, but there are no other numeric columns to compare against:

"No other numeric columns, (columns with more than 50% numeric data (green values)), to compare with Column $1."

image

@github-actions github-actions bot added Type: Bug Issues related to software defects or unexpected behavior, which require resolution. error handling Improving the ways errors are reported to users facets Behaviour or rendering of facets in a project labels May 26, 2022
Copy link
Member

@wetneb wetneb left a comment

Choose a reason for hiding this comment

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

Works for me, leaving open for longer if people want to chime in.

@ostephens
Copy link
Member

Looking at the code the test of whether a column is Numeric or not seems to be that more than half the values in the column are numeric. So two questions

  1. Are we happy with this as our threshold for allowing scatterplots? (I don't have any strong feelings as I never use this functionality!)
  2. And perhaps more importantly - should we be more explicit in the user feedback to state that this is the definition?

@wetneb
Copy link
Member

wetneb commented May 28, 2022

It is definitely not ideal to rely on such a threshold… I guess it would make sense to mention it in this error message indeed!

@elebitzero
Copy link
Member Author

I updated the error messages to mention the threshold, and I noticed the scatterplot documentation doesn't mention what a numeric column is: (Ref: https://docs.openrefine.org/manual/facets#scatterplot-facet). Scatterplot seems to be the only feature which uses the concept of a numeric column. Other features work on individual cells, like numeric facet.

@wetneb wetneb merged commit a6c395a into OpenRefine:master Jun 15, 2022
@elebitzero elebitzero deleted the blank-scatterplot-fix branch October 26, 2022 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Improving the ways errors are reported to users facets Behaviour or rendering of facets in a project Type: Bug Issues related to software defects or unexpected behavior, which require resolution.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scatterplot facet should show error on non-numeric column
3 participants