Skip to content

find_scale() works with scales from packages that are loaded but not attached #4799

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

krlmlr
Copy link
Member

@krlmlr krlmlr commented Apr 10, 2022

Closes #4705.

@thomasp85
Copy link
Member

What is the performance repercussions of searching all loaded namespaces? There are many standard aesthetics for which scales do not exist, because they are used as-is so find_scale() is called quite a lot where it will end up looking through all loaded namespaces

@krlmlr
Copy link
Member Author

krlmlr commented May 11, 2022

How many find_scale() are there in a usual plot? Should we avoid calling find_scale() for bare unclassed vectors?

Implementers can optimize by adding an "env" attribute to their scale_type() return value too.

@thomasp85
Copy link
Member

There will be one for every aesthetic not provided with an explicit scale in the plot construction... conserving this path for non-atomic aesthetics is probably a worthwhile addition...

Think about this, if we add it we should consider adding this approach to check_subclass() so that stats, geoms, and position can be looked up by name even in packages that are only loaded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support scale functions in packages not attached via scale_type()
2 participants