feat: Add "auto" unit to timeunit transform #3645
Open
+224
−29
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds a new "auto" unit option to the TimeUnit transform. When set to "auto", the transform will automatically choose the appropriate time unit based on the input data.
I wanted to try and solve the problem discussed in #1310: plotting discretized timeseries data without knowing the grain.
Here's a demo Observable notebook. It patches the timeunit transform and uses DuckDB to produce differently grouped time data.
Problem/Solution
Currently, you can discretize data into months using this transform.
But, if your data is already grouped by month, there's no way to infer that from the data.
This PR adds an "auto" unit option that would work with this example data.
Alternate approaches
In an issue comment, @jheer suggested creating a new discretizing time scale that acts like a band scale. I believe that discussion happened before the TimeUnit transform was added. Enhancing TimeUnit seemed more natural fit than adding a scale, but I'd love to hear any other thoughts! I meant this PR mostly as a discussion starter with some running code.
Questions
I'm uncertain about the
"units": ["auto"]
parameter. Would a separate param be better? e.g."inferUnits": true
or similar?I created a small npm package that does the work to determine the likely unit/step. It felt like a separate concern that's not specific to vega, but it's very small. Should I pull that into vega-time as part of this PR?
The main reason to leave it separate IMO is to add tests that would be annoying to have in Vega. I want to run the tests with Node booted into different timezones, but that's annoying requirement to put on a bigger project.