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

docs: Add initial date range to interval selection example #3667

Merged
merged 8 commits into from
Nov 3, 2024

Conversation

dsmedia
Copy link
Contributor

@dsmedia dsmedia commented Oct 31, 2024

This is a partial implementation of #3643, demonstrating how to set initial date ranges in interval selections through a concrete example. This improvement was made possible by the recent datetime support added in #3653 and #3662 by @dangotbanned.

Rather than creating a separate example, I opted to modify the existing interval selection example because:

  1. Setting initial ranges is a core feature of interval selections, not a separate use case. Including it in the primary example helps users discover this capability alongside basic usage.

  2. The change adds minimal complexity while showing the idiomatic way to set initial values using native datetime objects.

  3. A single progressive example provides a clearer learning path than splitting this into two separate examples.

Changes made:

  • Added demonstration of initial date range setting using datetime objects
  • Updated example title and description to reflect the addition
  • Added clarifying comments about the selection range

Remaining work in #3643:

  • Add information about initial ranges to the selection documentation
  • Consider documenting datetime handling in times_and_dates section

@dangotbanned dangotbanned self-requested a review October 31, 2024 14:57
@dangotbanned
Copy link
Member

@dsmedia thanks for opening this PR!

Rather than creating a separate example, I opted to modify the existing interval selection example because:

  1. Setting initial ranges is a core feature of interval selections, not a separate use case. Including it in the primary example helps users discover this capability alongside basic usage.
  2. The change adds minimal complexity while showing the idiomatic way to set initial values using native datetime objects.
  3. A single progressive example provides a clearer learning path than splitting this into two separate examples.

I agree with these points.
It makes more sense to me now that setting the value is simpler than the stackoverflow solution.


One question, do you think this example would make more sense in a different category?
I've noticed these two which have some similarities:

But they are catgorised as interactive charts.
This one is under area charts

There is also VL-interactive_overview_detail

I know that this wasn't a change that you introduced, but I think it would further improve discoverability of this functionality

Copy link
Member

@dangotbanned dangotbanned left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for the PR @dsmedia

I'm approving now, will leave open for a bit to see if anyone else wants to weigh in on the suggestions

tests/examples_arguments_syntax/interval_selection.py Outdated Show resolved Hide resolved
tests/examples_methods_syntax/interval_selection.py Outdated Show resolved Hide resolved
@mattijn mattijn enabled auto-merge (squash) November 3, 2024 20:55
@mattijn
Copy link
Contributor

mattijn commented Nov 3, 2024

Applied suggestions from review and activated auto-merge. Thanks @dsmedia! Much appreciated!

@mattijn mattijn merged commit 8fdd537 into vega:main Nov 3, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants