Doctest for GenericListArray.

[novemberkilo]

Which issue does this PR close?

Re #301

What changes are included in this PR?

Doctest for GenericListArray

Are there any user-facing changes?

No

[novemberkilo]

@alamb I can't tell if GenericListArray is something that folks would build directly so am not sure if I have chosen a good example or not. I've added a test as well. I could add another example based on a builder but wanted to please get your thoughts on this first. Thanks.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.65%. Comparing base (6e2f684) to head (9147c9a).
Report is 2894 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #474      +/-   ##
==========================================
- Coverage   82.65%   82.65%   -0.01%     
==========================================
  Files         165      165              
  Lines       45524    45524              
==========================================
- Hits        37628    37627       -1     
- Misses       7896     7897       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alamb]

Hi @novemberkilo -- I think users of Arrow would typically use ListArray or LargeListArray (which are both typedefs / implemented in terms of GenericListArray)

Maybe adding some documentation to GenericListArray hinting / directing users to ListArray and LargeListArray would be helpful (as this distinction / convention is not obvious in my opinion)

[novemberkilo]

Thanks @alamb - I will do so and add examples for ListArray and LargeListArray

[alamb]

Thanks @novemberkilo -- I have a small suggestion, but otherwise this is looking great.

Thank you

[alamb]

Suggested change

	/// Instead of using `GenericListArray` directly, consider using `ListArray` or `LargeListArray`
	/// Instead of using `GenericListArray` directly, consider using [`ListArray`] or [`LargeListArray`]

If you use [`ListArray`] instead, the docs will include a hyperlink to the class

[jorgecarleitao]

fwiw, I do not think this recommendation is fair. GenericListArray allows to write code generic over the Offset, which is more generic than only supporting ListArray or LArgeListArray.

The same applies for Utf8Array<T>, PrimitiveArray<T>, etc.

[novemberkilo]

Thanks @jorgecarleitao - I'm a little confused though. My initial commit attempted to show an example of this 4c5b2b4 which admittedly was the equivalent of the example for ListArray

Would you perhaps suggest different wording for this comment, or perhaps could you point me in the direction of a better example that we could use for GenericListArray ?

[jorgecarleitao]

I am sorry I was not very explicit. :(

The example is great, I would just omit the

Instead of using GenericListArray directly, consider using [ListArray] or [LargeListArray]`

which suggests that using ListArray is better than using GenericListArray, when imo we do not have enough information here to make this recommendation. In general using GenericListArray is better because it allows to write things like

fn some_f<O: ListOffset>(array: &GenericListArray<O>)

which makes it generic over both "ListArray" and "LargeListArray". Most code in arrow and DataFusion uses this to reduce code duplication while supporting both.

[novemberkilo]

Thanks - fwiw I was trying to tackle the specific problem of showing by an example, how to construct a GenericListArray

Per #474 (review) I removed the example for GenericListArray in favour of examples for ListArray and LargeListArray. The intent was not to suggest that using ListArray is better or worse than using GenericListArray

In any case, also removing the comment will result in no documentation or examples for GenericListArray

Is that the final state we want? Or should we have some documentation or hints for usage for GenericListArray

Hope that all makes sense. Thanks.

[alamb]

I think it is valuable if the comment on GenericListArray pointed (likely) novice users towards ListArray and LargeListArray as their existence may not be obvious at this stage

How about making the wording less judgemental such as

For non generic lists, you may wish to consider using [`ListArray`] or [`LargeListArray`]`

[alamb]

Thank @novemberkilo !