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

bail out if the inferred ordinal domain has more than 10k values #858

Merged
merged 5 commits into from
Jun 24, 2022
Merged

bail out if the inferred ordinal domain has more than 10k values #858

merged 5 commits into from
Jun 24, 2022

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Apr 29, 2022

@Fil Fil requested a review from mbostock April 29, 2022 20:34
mbostock
mbostock reviewed Apr 29, 2022
View reviewed changes
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

I think we only want this restriction for positional ordinal scales (point or band), not for all ordinal scales.

@Fil
Copy link
Contributor Author

Fil commented Apr 29, 2022

I've added this check. I realize I don't know why we're exporting ScaleO (which is not used anywhere else and not part of the API).

mbostock
mbostock reviewed Apr 29, 2022
View reviewed changes
src/scales/ordinal.js Outdated Show resolved Hide resolved
mbostock
mbostock reviewed Apr 30, 2022
View reviewed changes
src/scales/ordinal.js Outdated Show resolved Hide resolved
@mbostock mbostock merged commit 2cdaf18 into main Jun 24, 2022
@mbostock mbostock deleted the fil/vhcd branch June 24, 2022 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbostock mbostock mbostock 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.
2 participants
@Fil @mbostock