search

brownplt / code.pyret.org

Website for serving Pyret to folks.
Other
24 stars 45 forks source link

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

Open schanzer opened 8 months ago

schanzer commented 8 months ago

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... 😉 )

shriram commented 8 months ago

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

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

schanzer commented 8 months ago

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?
shriram commented 8 months ago

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.

asolove commented 8 months ago

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 commented 8 months ago

@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?

schanzer commented 8 months ago

@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! 🙏