-
Notifications
You must be signed in to change notification settings - Fork 794
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
Promote the use of alt.when().then().otherwise()
over alt.condition()
in the User Guide?
#3500
Comments
Thanks for the shoutout @joelostblom Only thing I wanted to add is I'm currently working on fleshing out the examples in #3492 There is quite a lot of functionality packed into the new methods, and from experience with polars.when I found it really quick to pick up learning this way |
@joelostblom the proposal here #3476 (comment) I see as helping get towards these being equal: alt.when(predicate).then(alt.value("red")).otherwise(alt.value("blue"))
alt.when(predicate).then("red").otherwise("blue") The second half of this comment shows the issues with assuming shorthand, but that could be resolved by requiring Long-term goal, but maybe something for the next breaking release |
Do you have any specific ideas on how to approach this?
I think the Examples section for I also left this comment that explains |
A few thoughts here:
|
@joelostblom really appreciate the detailed response!
I agree. I was about to suggest adding the callout at conditions-filters - which I now understand was what @mattijn meant in #3500 (comment)
Have a few thoughts on this. If at all possible, having
So in #3485 I added interactive_bar_select_highlight which has an example of this usage. It was just recreating a
This somewhat overlaps with (2), so if that introduces a subpage that might change my opinion here.
I think the reordering would be helpful, so that expressions are introduced before being used in data-driven-comparisons and encoding-channel-binding.
100% agree, if anything adding a tab for that would make it more difficult to understand |
(sorry this became longer than intended) 2. I generally prefer to include examples in the User Guide if there is a suitable location for them. I think this is one such case, so I would vote for moving the gallery example into the new section of the user guide instead. Your point about using the I think we can decide to either lean into the docstring examples, add them to all objects that don't have them currently, and then inline the docstring examples in the relevant sections of the User Guide. Pros: easy to view example right in the docstring, only update in one place (although some User Guide sections are too long to include in the respective object's docstring: e.g. the section on Another option is to focus on having the example code only in the User Guide (maybe with a link in the docstring to the relevant section in the docs?). Pros: Closer to the existing approach so there is less to update, charts only show up online anyways, Cons: Not possible to view example directly in the help pop up in IDEs. A third approach would be something in between with just a simple example in the docstring and more elaboration in the User Guide. Pros: There is both something directly in the docstring and also an elaborate version in the User Guide. Cons: We're now updating in two places, but maybe the docstring example can be kep so simple that it doesn't matter much. There is also a larger discussion around what goes into the Gallery and what doesn't. We don't want it too cluttered, but it is also a popular page to visit. We could probably remove some examples from the Gallary and also add links from the Gallery to the relevant sections in the User Guide so that people find them too (but this is a separate PR). 3. I vote that we split the interactive docs and introduce the examples you have created for The reordering of 2 and 3 is not intuitive for me as we make quite heavy use of widgets when explaining the expressions (and I don't see us using expressions much in the widget section). Don't you think they would need to see widgets before understanding the expression section? |
@joelostblom really sorry for missing #3500 (comment), I seem to have dismissed the notification via email? 🤦♂️ Will do my best to get back this soon, but thank you for taking the time to write it! |
@joelostblom finally got back to this.
I've seen diataxis mentioned in
So I definitely appreciate examples in docstrings, but would be hesitant in adding them to autogenerated docstrings. Also if they are not written by hand, I expect they will be of low utility
Of the options presented, this would be my preference with ... Additional ProposalI'm viewing examples less in terms of simple vs complex, and more in terms of serving the context they are displayed in. Docstrings/API Reference
User Guide
Gallery
If you have the time, would you be able to make the split (either as its own PR or in #3544)? I am happy to do the
I'll defer to you on this as I haven't used expressions in much detail and you seem much more knowledgeable on them than I. |
Finally found waterfall chart example. I recalled seeing this comment but couldn't find it when searching:
This should be re-written with |
Good reminder! I also think this could be helpful in the larger picture, but it's a bigger project for me to understand diataxis more intimately and think about if it would serve us to (iteratively) move towards that. But I had a glance at the page and as per https://diataxis.fr/compass/ and https://diataxis.fr/complex-hierarchies/ maybe our current docs map onto diataxis something like this (mostly for future reference):
I also think it would be helpful with something simple like more (two-way) links between the User Guide and the Gallery examples to guide readers toward more relevant information.
I agree with this.
I find this framing and the structure in your additional proposal helpful. Let's proceed with that framework as our guide for deciding where to put examples and what they should look like.
Yup I can put up a separate PR.
Agree! I can also update https://stackoverflow.com/questions/66108224/combine-hover-and-click-selections-in-altair/66109641#66109641 once there is a doc section to link too |
What is your suggestion?
After all the fantastic work of @dangotbanned , we now have
alt.when().then().otherwise()
as an alternative toalt.condition(condition, if_true, if_false)
. This new syntax allows for multiple predicates to be specified, which is not possible withalt.condition
, looks more idiomatic (in line with our method chaining for encoding channel options), and is similar in length to type toalt.condition
(but might be shorter in the future if it becomes possible to pass string literals in an unambiguous manner).When we introduced the method based syntax for channel options, we updated the docs everywhere and added a note in https://altair-viz.github.io/user_guide/encodings/index.html#channel-options
Should we do the same for
alt.when().then().otherwise()
or are there any reasons to keepalt.condition
as the main conditional syntax in the docs?Have you considered any alternative solutions?
If we keep
alt.condition
the favored option in the documentation, we should at least add a section on how to usealt.when...
in the User Guide.Regardless of how we decide to update the docs, I think we should also explicitly mention the use of
empty
inside the conditional, as opposed to in the parameter definition (#3490).The text was updated successfully, but these errors were encountered: