Vega-lite: groupby in SummarizeTransform is not camelCase (groupBy)

[mattwchun]

[domoritz]

It isn't in Vega either: https://vega.github.io/vega/docs/transforms/aggregate/. However, timeUnit is camel case in vega-lite. But maxbin is not...

We should be consistent and break it now rather than after 2.0.

[jheer]

For the record, and with little rational reason, I'm not a fan of camel case. I'm even less a fan of breaking things others have become accustomed to.

[kanitw]

For the record, and with little rational reason, I'm not a fan of camel case. I'm even less a fan of breaking things others have become accustomed to.

Yeah, but timeUnit and many scale, axis, legend, and config properties all use camel case.
For the new properties (e.g., groupBy in the new SummarizeTransform for transform array),
we definitely should be consistent.

(I'm not asking to breaking these in Vega as the number of users who use groupby in Vega would
be way larger.)

For maxbins in Vega-Lite, I'm also slightly leaning toward breaking it now for consistency too -- although I agree it's a bit annoying to introduce breaking changes. (If we gonna break this in Vega-Lite, now is the time -- Another option is to break it in the schema but offers backward compatibility in the compiler.)

[domoritz]

I vote for breaking it now and consistently use no camel case. Do we have other examples besides timeUnit?

And it's only this one case, we can make it work in the compiler.

[kanitw]

many scale, axis, legend, and config properties

There are too many things to break (all of multi-word properties are camelCase).
I would say keep the camelCase so we only break a few things.

Note that most of these properties also have camel case in Vega.

[domoritz]

Note that most of these properties also have camel case in Vega.

Oh okay. I just randomly looked around and saw a lot of things without camel case.

[kanitw]

I just randomly looked around and saw a lot of things without camel case.

What are the things without camel case besides maxbins, zindex, and groupby?

[kanitw]

Oh gosh -- vconcat, hconcat.

[kanitw]

(vconcat and hconcat are new things though.)

[domoritz]

What are the things without camel case

modeskew, minstep maybe

[kanitw]

Yeah, minstep is a bin parameter like maxbins.

However, modeskew is value of a property (aggregate), not a key so it does not count.

That said, for many property values, we actually use dash.
Example:

• box-plot,
• all of the interpolate values such as linear-closed

[domoritz]

Okay, I created https://github.com/vega/vega-lite/pull/2404 to fix hconcat and vconcat. I think the others are not necessary as groupby is a known keyword in SQL, zindex is almost a common noun, and maxbins is also a short keyword (and we are consistent with Vega).

[kanitw]

In SQL, it's GROUP BY (with space in between) though.

[kanitw]

maxbins is also a short keyword (and we are consistent with Vega).

I wonder whether consistency within language is more important than consistency with Vega.

[domoritz]

In SQL, it's GROUP BY (with space in between) though.

Yes, but even pandas made it a single word: https://pandas.pydata.org/pandas-docs/stable/generated/pandas.DataFrame.groupby.html

[kanitw]

zindex is almost a common noun

Yeah, I'm fine with this one. zIndex looks weird after all.

In fact, hConcat looks weird to me too. That's probably because camelCase looks nicer when the prefix is a word, not a single character. (So I don't know if we should merge #2404. -- wanna see what @jheer and @arvind says).

In SQL, it's GROUP BY (with space in between) though.

Yes, but even pandas made it a single word: https://pandas.pydata.org/pandas-docs/stable/generated/pandas.DataFrame.groupby.html

Panda seems to use lowercase for multi-word methods (e.g., applymap).
Thus, I don't think they consider groupby a single word -- it looks more like a lowercase multi-word method name to me.

[g3o2]

Consistency with vega is certainly important, as is consistency with d3, svg and css where applicable. Consistency inside of vega-lite should come last IMO.

[kanitw]

Consistency with vega is certainly important, as is consistency with d3, svg and css where applicable. Consistency inside of vega-lite should come last IMO.

@g3o2 Why do you think that is the case? One important thing to consider is that more users of Vega-Lite will have no prior experience on Vega and D3. Thus, it is more likely that users will learn that most properties have camel case (from within the language) and run into a surprise that a few of them use lower case.

Also worth noting that this camel case issue is a bit delicate as we are consistently inconsistent (consistent with Vega, but Vega is inconsistent.)

[jheer]

To add clarity to my earlier (glib) comment: I think we should use camel case when the semantics warrant. For example strokeWidth, strokeDash, _etc_ are good cases. There are two logical components here: the aspect being modified (stroke) and the specific property thereof (width, dash, etc).

I think it is fine _not_ to use camel case for single noun or verb-like directives; zindex and groupby are examples where I would prefer not to use camel case.

An example in the middle of the road is something like maxbins. Based on my own logic above, I think it is reasonable to argue that it could be maxBins, or, if one wants more clarity at the cost of a few characters, maxBinCount (i.e., to avoid confusion with the value range being binned).

In general, I certainly value consistency, but not necessarily to the point of breaking existing conventions when the consistency benefit is negligible.

[kanitw]

In general, I certainly value consistency, but not necessarily to the point of breaking existing conventions when the consistency benefit is negligible.

I agree with this point. Thus, I think maxbins, minsteps, zindex should stay the same.

For groupby of summarize transform, it is a new property that we are adding so it is not so much about "breaking existing conventions" so we have more freedom to choose whatever we see appropriate. From googling around a bit, it seems like most API treat group by as two words (although some languages like Python use lowercase for two-word methods). So I am not entirely convinced that it is really a noun-like directive. That said, if you all think it is really a noun, I can close #2406.

@jheer -- what about v/hconcat? I feel like hConcat, vConcat look weird (but maybe it's okay).

[kanitw]

Oh I think I see what @jheer means now. groupby is a verb-like, not a single-word noun.
Using this same logic, vconcat and hconcat are also verb-like, so they should also have lowercase?

[jheer]

Yes, let's keep hconcat and vconcat as single lower case terms. And, yes, I think of these and groupby as single verb-like directives.

I'm OK changing maxbins to maxBinCount if others agree.

[kanitw]

I'm OK changing maxbins to maxBinCount if others agree.

If we do that, bin's minstep should become minStep too?