-
Notifications
You must be signed in to change notification settings - Fork 45
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
Some chart functions run out of colors (likely array index issue) #507
Comments
Perhaps this is of use, since it's in the codebase already…
https://blog.brownplt.org/2018/06/11/philogenic-colors.html
|
That's a great blog post. Two nits:
|
Sorry, not sure. No real time to find it. I'd say look for the place in the
code where error messages are generated, it's got to be referenced from
there.
|
I'm looking into this. For reference: the color generation code used for error highlighting, under discussion above, is here: code.pyret.org/src/web/js/output-ui.js Line 47 in 1701bc6
|
@schanzer I can reproduce this in production, but am not able to get a similar |
@asolove of course! The entire context can be found here, but what matters is the definition of Thank you for looking into this! 🙏 |
I think this issue exists in most of our chart functions, but some are more resilient than others.
In this starter file, we import a large dataset. Try evaluating
stacked-bar-chart(shark-attacks-table, "ATTACK-TYPE", "COUNTRY")
, and you'll see that we quickly run out of colors for all the countries....and default to black.I believe one example of the bug can be traced to this line in bar charts, where we naively try to get a slice of the
color_list
object using the number of legends as our slice length. $10 says that when we access the color at indexreallyBigNumber
, the resultingundefined
gets turned into a pyret color with zeros for all the fields. In fact, this will likely happen atnotSoBigANumber
, because the default color list is pretty short!I think the general solution here is that all charts should cycle through their color lists if they need more colors than the list allows. Bonus points if they start tweaking the colors programmatically. I would love to send a PR for this myself, but I'm overloaded with curriculum and PD staffing right now. :(
(Could potentially be a great target for @asolove , if he's got an afternoon to kick around... 😉 )
The text was updated successfully, but these errors were encountered: