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

_PropertySetter("title", ...) dynamic doc uses wrong class #3122

Open
franzhaas opened this issue Jul 23, 2023 · 8 comments · Fixed by #3660
Open

_PropertySetter("title", ...) dynamic doc uses wrong class #3122

franzhaas opened this issue Jul 23, 2023 · 8 comments · Fixed by #3660
Labels
bug has-repro Issues with a Minimal, Reproducible Example

Comments

@franzhaas
Copy link
Contributor

franzhaas commented Jul 23, 2023

Dear all,

I tried the example from https://altair-viz.github.io/gallery/layered_chart_with_dual_axis.html, both the Method syntax as well as the Attribute syntax.

On my setup (jupyterlab 4 + altair 5.0.1) only the attribute syntax colours the axis labels with the colours from the graph. The Method syntax colors the axis labels black.

I am happily using the Attribute syntax now. Is this a bug? Of the documentation? Of the library itself?

Thanks in advance,
Franz


Minimal Repro

@franzhaas franzhaas added the bug label Jul 23, 2023
@joelostblom
Copy link
Contributor

Thanks for reporting, the reason is that Vega-Lite's title parameter only takes a string, so the title() method (or title= parameter in version <=4) can only take a string of the title text and no parameters for customizing the title (so other parameters such as subtitle= also does not work inside title(). This is currently not clear from the docstring which mentions that it it possible to pass parameters to title, so we should either change the docstring or silently pass those parameters forward to axis(title={}).

Shorter examples:

import altair as alt
from vega_datasets import data

source = data.seattle_weather()


alt.Chart(source).mark_point().encode(
    alt.X('date').title('This should be red', color='red')
)

image

alt.Chart(source).mark_point().encode(
    alt.X('date').axis(title='This should be red', titleColor='red')
)

image

@franzhaas
Copy link
Contributor Author

Ok, thanks this makes sense now...

@joelostblom joelostblom changed the title dual y axis examples .title() does not support all the argument in axis(title=...) but still lists them in the docstring Sep 21, 2023
@franzhaas
Copy link
Contributor Author

I just checked, it is still an issue with 5.2.0.

@joelostblom
Copy link
Contributor

Yes, this issue will be closed once a PR has been submitted to fix it. Feel free to create one if you have the time to get familiar with the code base.

@dangotbanned
Copy link
Member

Closing as #3660 addresses this example

@joelostblom
Copy link
Contributor

@dangotbanned I'll reopen this because it is tracking the general feature of being able to use additional parameters inside .title() just as they can be used inside .axis(title=, ...)

@joelostblom joelostblom reopened this Oct 29, 2024
@dangotbanned
Copy link
Member

@dangotbanned I'll reopen this because it is tracking the general feature of being able to use additional parameters inside .title() just as they can be used inside .axis(title=, ...)

@joelostblom that's fine with me. Has there been an issue opened upstream for this?

IIRC we would have support for this added "for free" if the vega-lite schema specified TitleParams as an option that is allowed here

@joelostblom
Copy link
Contributor

Has there been an issue opened upstream for this?

I was going to add one right now, but I realized that in the VegaLite spec, maybe this is not much of an advantage as it would be in Altair. E.g. take this spec:

"x": {"field": "date", "title": "A title text", "type": "temporal"}

With out suggestion we are saying it should be possible to pass a dictionary to the "title" key, but that is not too different from what can already be done with axis since there are no positional params in VegaLite's json spec:

# These two options are equally convenient
"x": {"field": "date", "title": {"title": "A title text", "color": "red"}, "type": "temporal"}
"x": {"field": "date", "axis": {"title": "A title text", "color": "red"}, "type": "temporal"}

The difference is more notable in altair since the actual text of the title is passed as a positional argument:

alt.X('date').title("A title text", color="red")
alt.X('date').axis(title="A title text", color="red")

Having that said, it is not a major advantage and I think the biggest issues is that altair suggests it is possible in the docstring to use all the parameters with .title() directly although it isn't:

Image

I see two paths:

  1. Fix the .title docstring to indicate that it only takes a string and point users to .axis(title=) for more customization.
  2. Do some preprocessing of the schema to always pass everything inside .title() to .axis(title=...) where the first positional argument will be the text of the title.

The second one would be nice, but if the first one is less work then that is a perfectly fine option to me.

@dangotbanned dangotbanned added the has-repro Issues with a Minimal, Reproducible Example label Jan 10, 2025
@dangotbanned dangotbanned changed the title .title() does not support all the argument in axis(title=...) but still lists them in the docstring _PropertySetter("title", ...) dynamic doc uses wrong class Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug has-repro Issues with a Minimal, Reproducible Example
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants