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

Improve docs for shape and a la carte shaping functions #5172

Merged
merged 3 commits into from
Jul 8, 2024

Conversation

philrz
Copy link
Contributor

@philrz philrz commented Jul 7, 2024

When it comes to working with records, we've had some internal consensus that users will probably prefer shape most of the time whereas the "a la carte" functions cast, fill, and order are probably going to see more limited use. Addressing the questions posed by the community user that opened #5168 made me realize this is not really called out in the docs, so I'm attempting to do that here, along with a couple other bits of polish I saw in the pages along the way.

@philrz philrz requested a review from a team July 7, 2024 21:09
@philrz philrz self-assigned this Jul 7, 2024
Comment on lines 19 to 21
The order of newly added fields relative to fields already present is
undefined. If maintaining relative order is important, consider applying the
[`order`](./order.md) or [`shape`](./shape) function.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I stuck with "undefined" since I don't know the code. If we think there's something more detailed we could say that's not too complicated I'd be happy to set more explicit expectations (e.g., if/when it can be expected to append the new fields).

Comment on lines 19 to 21
The order of newly added fields relative to fields already present is
undefined. If maintaining relative order is important, consider applying the
[`order`](./order.md) or [`shape`](./shape.md) function.
Copy link
Member

Choose a reason for hiding this comment

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

Fields are added in the order they appear in the input record.

Copy link
Member

Choose a reason for hiding this comment

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

For reference, fill essentially happens here:

https://github.com/brimdata/zed/blob/f497079907a9ffcb46d31eb7828d39357cc8a211/runtime/sam/expr/shaper.go#L274-L275

(That's inside a loop that, for fill, iterates over the fields of the input record.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After seeing @nwt's comments here we had a quick offline chat and agreed on some text to clarify where fill adds the fields rather than the "undefined" I had before. I've pushed a commit with that change.

@philrz philrz merged commit b9ec428 into main Jul 8, 2024
4 checks passed
@philrz philrz deleted the shape-docs-clarify branch July 8, 2024 18:53
@philrz philrz linked an issue Jul 14, 2024 that may be closed by this pull request
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.

casting complex type is order dependent when docs say different
2 participants