New time_pivot visualization #3941
Conversation
mistercrunch
force-pushed
the
time_pivot
branch
from
c90ac87
to
527b2c0
Compare
|
@williaster @graceguo-supercat I'd love a review on this pretty plz |
|
sorry will 👀 ! |
| clearable: false, | ||
| choices: [ | ||
| ['AS', '[AS] Year'], | ||
| ['52W-MON', '[52W-MON] 52 Weeks (starting Monday)'], |
There was a problem hiding this comment.
should there be an equivalent for 52 Weeks (starting Sunday)?
There was a problem hiding this comment.
Seems reasonable to add in the default list. It's just hard to know where to stop (other days of the week?). My intent was to show a few example expression and let the users compose their own following the pattern. I'll clarify the help text and add more examples.
There was a problem hiding this comment.
Oh I see, yeah no need to add. I thought you were maybe doing the defaults based on known use-cases as opposed to generic variety so I think not adding is 👍
| ['4W-MON', '[4W-MON] 4 Weeks'], | ||
| ], | ||
| description: t( | ||
| 'The frequency over which to pivot time. Free-form "pandas" offset alias ' + |
There was a problem hiding this comment.
you can use text ticks `text` and wrap without concatenation (it inserts spaces for newlines)
There was a problem hiding this comment.
oh I didn't know ticks did that. neat
superset/viz.py
Outdated
| @@ -1136,6 +1146,47 @@ class NVD3TimeSeriesBarViz(NVD3TimeSeriesViz): | |||
| verbose_name = _('Time Series - Bar Chart') | |||
|
|
|||
|
|
|||
| class NVD3TimePivotViz(NVD3TimeSeriesViz): | |||
|
|
|||
| """Overlapping time series""" | |||
There was a problem hiding this comment.
"Overlayed" maybe seems more accurate?
| freeForm: true, | ||
| clearable: false, | ||
| choices: [ | ||
| ['AS', '[AS] Year'], |
There was a problem hiding this comment.
what does AS mean here?
EDIT: I see it's pandas jargon. Can you just encode AS as the value and remove AS just using year in the label? novice users are not going to understand that and even though it says year currently they might think it's doing something else confusing and be unsure of their selection because it says AS
There was a problem hiding this comment.
I get that it's confusing, and was trying to be transparent about the pandas freq mapping. Especially given that this is a freeform dropdown allowing them to use whatever valid pandas freq. Maybe moving to Year (freq=AS) and following that pattern?
There was a problem hiding this comment.
I agree transparency is valuable. I think that pattern is a good middle ground!
| @@ -195,6 +195,50 @@ export const visTypes = { | |||
| }, | |||
| }, | |||
|
|
|||
| time_pivot: { | |||
| label: t('Time Series - Period Pivot'), | |||
There was a problem hiding this comment.
I think the terminology for this visualization is confusing, you intermix "period" and "frequency" a lot and I think both of those are confusing for people not already familiar with more complex data analysis.
What do you think of calling this Time Series -- Seasonality Pivot and using seasonality consistently? If you don't like that, I think periodicity is next best.
cc @elibrumbaugh @graceguo-supercat for thoughts!
There was a problem hiding this comment.
Very good point. We're planning to use this mostly on periodicity of week and day at Lyft, so I prefer "periodicity" to "seasonality" which seems to imply year...
There was a problem hiding this comment.
I agree, "seasonality" does seem most associated with year although it doesn't have to be. "periodicity"++
|
Adressed comments, mergin. |
* New time_pivot visualization * Minor tweaks * Addressing comments
* New time_pivot visualization * Minor tweaks * Addressing comments
mistercrunch
added
🏷️ bot
No description provided.