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

MarkDef docs show wrong docstring #3370

Closed
joelostblom opened this issue Mar 16, 2024 · 2 comments
Closed

MarkDef docs show wrong docstring #3370

joelostblom opened this issue Mar 16, 2024 · 2 comments
Labels

Comments

@joelostblom
Copy link
Contributor

Follow up on #3266

The docs on MarkDef show that it takes the composite mark strings 'boxplot" etc as arguments, but this is not correct. Those need to be passed to CompositeMarkDef instead. The type hint is correct and does not include the composite marks:

image

I think the docstring is fetched from mark.ts in Vega-Lite, where there is only a generic definition, so I'm not sure that we can actually separate it on the Altair level. We might even argue we don't need to list the individual strings now that they are already included in the type hint (but again, not sure how to exclude that from being fetched from VL).

@binste
Copy link
Contributor

binste commented Mar 17, 2024

I think that this is ideally changed in VL so that we can just can keep taking these descriptions from the VL schema. Also, the VL schema itself would also be improved by it.

Docstring seems to come from that place you linked to and is then used for MarkDef here and for CompositeMarkDef here. There should probably a separate docstring for these two so that it matches with the type hints. Would you be ok if we move this to the VL repo?

@joelostblom
Copy link
Contributor Author

Great, I didn't see that there were separate definitions of MarkDef and CompositeMarkDef; hopefully it can just be changed in each one of those classes then. I couldn't transfer to another org so I opened vega/vega-lite#9290

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

No branches or pull requests

2 participants