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.

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

Enforce that all axis types are <: AbstractAxis #564

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

asinghvi17
Copy link
Member

As brought up in Slack just now. Still needs tests but works locally with GeoAxis.

One question that remains is what to do about polar axes. With the naive implementation here, draw(; axis = (; type = PolarAxis)) fails because the constructor tries to pass xlabel etc in the kwargs, which PolarAxis can't handle.

@asinghvi17
Copy link
Member Author

Bump - is the best thing to do just a translate_to_axis(::AxisType, attrs) function or something?

@jkrumbiegel
Copy link
Member

xlabel and ylabel just happen to be the names for dim1 and dim2 in Axis, for PolarAxis it's different. So maybe instead of translating from xlabel a previous step would just pick the correct label for a different type. One thing that will also be a bit wrong is that the aesthetics are called AesX and AesY. So then you'd have to do scales(X = ...) also for PolarAxis

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.

2 participants