vega / altair

Declarative statistical visualization library for Python
https://altair-viz.github.io/
BSD 3-Clause "New" or "Revised" License
9.39k stars 795 forks source link

feat: Generate docstrings for `mark_` methods #3675

Closed dangotbanned closed 2 weeks ago

dangotbanned commented 2 weeks ago

Closes #3262

Description

This PR is a 2-for-1.

  1. Primarily #3262
  2. But also reduces mixins.py from 3500 to 1600 lines

What I've done here is created a dummy version of each class, where the only difference is the omission of type. The dummy is used only for the signature & docstring.

If we used core.MarkDef directly, it always displays type first. I'm assuming that wouldn't be desirable - since we set that explicitly in the method body

Example

IDE

https://github.com/user-attachments/assets/83d8f08b-9ce5-4856-8a5b-9e52e8a72f20

API reference

Remains unchanged

![image](https://github.com/user-attachments/assets/8d8ebbac-d025-4b54-a3a3-9fd352bb9dbc)

mattijn commented 2 weeks ago

If I recall correctly, not every mark supports the same variable definitions, do you assume here that all marks can use the same MarkDef? Btw, I've no supporting materials yet that supports my assumption.

dangotbanned commented 2 weeks ago

@mattijn thanks for looking at this so soon!

If I recall correctly, not every mark supports the same variable definitions, do you assume here that all marks can use the same MarkDef? Btw, I've no supporting materials yet that supports my assumption.

Just to be sure I understand what you mean, are you referring to this old comment?

https://github.com/vega/altair/blob/c1e806223fdd781b28e2aa0b3f7440e3431a001e/tools/generate_schema_wrapper.py#L920-L926

I've interpretted that as meaning how the docs for MarkDef has some descriptions which state they only apply for specific marks (e.g. x2 applies to {"area", "bar", "rect", "rule"}).

If so, I don't think there would be a reliable way to do that - since the descriptions aren't written in a consistent style.


Or did you mean core.MarkDef vs the composite marks?

https://github.com/vega/altair/blob/c1e806223fdd781b28e2aa0b3f7440e3431a001e/altair/vegalite/v5/schema/mixins.py#L698

https://github.com/vega/altair/blob/c1e806223fdd781b28e2aa0b3f7440e3431a001e/altair/vegalite/v5/schema/mixins.py#L817

https://github.com/vega/altair/blob/c1e806223fdd781b28e2aa0b3f7440e3431a001e/altair/vegalite/v5/schema/mixins.py#L892

If so, then yeah these are split out correctly

mattijn commented 2 weeks ago

I mean the latter. Good to see that the composite marks are still treated differently, but I thought that not all arguments can be used within the standard marks. It seems that it was and will be covered by just the MarkDef.

mattijn commented 2 weeks ago

Thanks!

dangotbanned commented 2 weeks ago

Thanks!

Appreciate the review thanks @mattijn