Should we change the Altair docs to use a more conventional method chaining syntax?

[joelostblom]

This has come up a few times in various issues, so I'm just making a single discussion for it here. Currently the altair documentation uses a less conventional syntax for method chaining:

alt.Chart().mark_point().encode(
    x=...
    y=...
).more_methods(
    param=value
).even_more_methods(
    param1=value1,
    param2=value2
)

It seems to be more popular in other libraries to use wrapping parens instead (e.g. pandas, polars, seaborn, ibis):

(
    alt.Chart()
    .mark_point()
    .encode(
        x=...
        y=...
    )
    .more_methods(param=value)
    .even_more_methods(
        param1=value1,
        param2=value2
    )
)

In addition to being more easily recognizable by folks new to the library, the second approach also has the advantage of being correctly autoformatted by ruff and similar formatters (and maybe it is clearer to read with exactly one method per line?). At the same time I have found the current approach quite convenient to type as I don't have to go back to add a parenthesis and I sometimes use it for other libraries as well (although I remember initially finding it confusing so it might just be me getting used to it). I see advantages of both approaches and the current one has been around for quite some time, although if I were to write a new library today I would probably opt for the second approach for consistency with other libraries and formatters.

A few places where this has come up:

• Docs: Add another version of the 'Multiline Tooltip' exmaple which uses the standard tooltip #3340 (comment)
• docs: Add example for *Bar Chart with Highlighting on Hover and Selection on Click* #3485 (comment)
• docs: Promote when-then-otherwise in User Guide #3544 (comment)

[joelostblom]

While we are talking about changing conventions, there are also other things we can consider for clarity, e.g. maybe we should always write transform before encode since the transform operations happen before encode sees the data and can change which data fields are visible to the encode method. Maybe also encode before mark since that seems more in line with how we think about the data (ie first which variables should go in the chart, then which mark to use to represent them graphically (this is how it is done in ggplot and was originally the case in altair but changed early on for consistency with VL #93 (comment)))

[dangotbanned]

Related

• Formatter: `np.array` formatting astral-sh/ruff#8452 (comment)
• ☂️ Formatter: Support formatting of embedded code astral-sh/ruff#8237
  • ruff currently doesn't support .rst
• https://github.com/adamchainz/blacken-docs
  • AFAIK, the common alternative for that
• https://docs.astral.sh/ruff/configuration/#config-file-discovery
  • You could use an alternative pyproject.toml for these dirs
    • https://github.com/vega/altair/tree/main/tests/examples_arguments_syntax
    • https://github.com/vega/altair/tree/main/tests/examples_methods_syntax

[binste]

I'd be in favour of using whatever ruff does. +1 to all the advantages you listed. It's also the code style that users are likely to know from their own code bases where the formatters black and ruff (very similar in style for the purposes of this discussion) are ubiquitous.

At the same time I have found the current approach quite convenient to type as I don't have to go back to add a parenthesis and I sometimes use it for other libraries as well

The beauty of a code formatter would be that you can still use the current approach, but when saving the file/creating a PR/..., ruff takes care of this for you :) So everyone can still write however they want, but in the end it looks the same.

[joelostblom]

I see the point that it more resembles what users are writing, but once they save and have autoformatting enabled (also in notebooks), then they see it the second way so I'd expect that many are used to both ways and are aware that this is purely formatting.

I wonder how common it is to use automatic formatters, particularly among newer coders/data scientists who are probably those that struggle the most when trying to learn a new library. I guess we could always encourage it briefly somewhere in the docs. As long as the autoformatted code is still convenient to write manually it's OK, which I think is the case here.

One of the most appealing aspects to me with moving to a more commonly adopted style is that we adhere as much as possible to a method chaining standard, which means that skills/conventions can transfer between libraries and we contribute towards a common solution to a shared problem.

For jupyter, there is https://jupyterlab-code-formatter.readthedocs.io/index.html, but I haven't used it myself. I also seems like Ruff might not always reformat method chains in our preffered way (astral-sh/ruff#8598), but that's minor. It also only kicks in if there is a single too long line if I'm not mistaken, so it won't work to always write the current style and then rely on ruff to reformat it, e.g. this is not reformatted for me:

import altair as alt
from vega_datasets import data

source = data.iris()

alt.Chart(source).transform_fold(
    ["petalWidth", "petalLength", "sepalWidth", "sepalLength"],
    as_=["Measurement_type", "value"],
).transform_density(
    density="value", bandwidth=0.3, groupby=["Measurement_type"]
).mark_area().encode(
    alt.X("value:Q"), alt.Y("density:Q"), alt.Color("Measurement_type:N")
)

Another issue is that if I put alt.X(), alt.Y(), and alt.Color() on separate lines, the automatic formatting will bring them together in one line. Maybe this is configurable somewhere, but I couldn't find a "one argument per line" rule or a "don't join multiple lines" rule when looking briefly.

Overall I think I'm mostly in favor of switching, but I would like to try out writing the style manually for a bit first to see how it is.

If this isn't handled automatically, then I see it mostly as a maintenance burden that I'd prefer to avoid

I agree that from a maintainer's perspective, having autoformatting is more convenient.

[dangotbanned]

, the automatic formatting will bring them together in one line. Maybe this is configurable somewhere, but I couldn't find a "one argument per line" rule or a "don't join multiple lines" rule when looking briefly.

@joelostblom I'm on my phone, but the feature is called "magic trailing comma" IIRC

[joelostblom]

Oh, that's great, thank you! Turns out it is actually the default, so adding a comma on the last parameter line prevents them from being joined. https://docs.astral.sh/ruff/settings/#format_skip-magic-trailing-comma

[dangotbanned]

That's the one!

There is also https://docs.astral.sh/ruff/rules/missing-trailing-comma/ which you could use to enforce it on certain files/globs

There should be a section in our pyproject.toml with an inverted per-file-ignores as an example

Apologies for the lack of specifics, hopefully that is enough to put you on the right track

[dangotbanned]

This isn't a priority for me, but trying to show an effort was made towards #3544 (comment)

The relevant changes you'd need:

diff --git a/pyproject.toml b/pyproject.toml
index 21c1a134..4be9c7d0 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -201,8 +201,6 @@ exclude = [
     ".git",
     "build",
     "__pycache__",
-    "tests/examples_arguments_syntax",
-    "tests/examples_methods_syntax",
     "tests/test_transformed_data.py",
 ]
 
@@ -327,6 +325,8 @@ select = [
     "I001",
     # complex-structure
     "C901",
+    # missing-trailing-comma (Only in examples)
+    "COM812"
 ]
 ignore = [  
     # Whitespace before ':'
@@ -403,6 +403,17 @@ See https://github.com/vega/altair/pull/3449
 # Allow complex if/elif branching during tests
 "tests/**/*.py"= ["C901"]
 
+# Always add trailing comma in examples https://github.com/vega/altair/discussions/3570#discussioncomment-10572821
+"!tests/examples_{arguments,methods}_syntax/*.py" = ["COM812"]
+
+
+"tests/examples_{arguments,methods}_syntax/*.py" = [
+    "D", # docs not relevant
+    "E711", # compare to None
+    "B018", # ignore for printing chart from variable
+    "NPY002", # too many numpy ones to fix manually
+]
+
 
 [tool.ruff.format]
 quote-style = "double"

The result is much closer to the original than using our existing config