code.pyret.org icon indicating copy to clipboard operation
code.pyret.org copied to clipboard

Some chart functions run out of colors (likely array index issue)

Open schanzer opened this issue 1 year ago • 7 comments

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 index reallyBigNumber, the resulting undefined gets turned into a pyret color with zeros for all the fields. In fact, this will likely happen at notSoBigANumber, 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... 😉 )

schanzer avatar Feb 07 '24 03:02 schanzer

Perhaps this is of use, since it's in the codebase already…

https://blog.brownplt.org/2018/06/11/philogenic-colors.html

shriram avatar Feb 07 '24 10:02 shriram

That's a great blog post. Two nits:

  1. It's "phillotactic", not "philogenic", right?
  2. There's no documentation of how a developer might use this infrastructure. If it's not listed in the API, can you give us a pointer to where it is in the codebase?

schanzer avatar Feb 07 '24 16:02 schanzer

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.

shriram avatar Feb 09 '24 04:02 shriram

I'm looking into this.

For reference: the color generation code used for error highlighting, under discussion above, is here: https://github.com/brownplt/code.pyret.org/blob/1701bc61e67e04342cb9d68c30a6a70d552aa50e/src/web/js/output-ui.js#L47

asolove avatar Feb 10 '24 23:02 asolove

@schanzer I can reproduce this in production, but am not able to get a similar stacked-bar-chart locally. Can you share with me (by email if sensitive) the content that's in the context used in that file, so I can call stacked-bar-chart locally?

asolove avatar Feb 11 '24 00:02 asolove

@asolove of course! The entire context can be found here, but what matters is the definition of stacked-bar-chart on line 441.

Thank you for looking into this! 🙏

schanzer avatar Feb 11 '24 00:02 schanzer