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

docs: fix and simplify logic that generates extension options docs #761

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ingomueller-net
Copy link
Contributor

This PR fixes the logic in generate_function_docs.py that generates the docs for the options of functions. The previous version mixed up the option names and the corresponding choices, leading to non-sensical docs such as reported in #755. This PR collects the option names and corresponding choices jointly such that no mix-up can happen. I have looked at the output that is changed by this script but only superficially so: my impression is that (1) some options now come in a different order (because the previous order was determined by the order of keys in a dict, which is implementation-defined) and (2) the remaining changes come from the intention of this PR, i.e., the constitute fixes of previously non-sensical output. However, since I am not intrically familiar with all extension functions, I am not 100% sure.

This resolves #755.

This PR fixes the logic in `generate_function_docs.py` that generates
the docs for the options of functions. The previous version mixed up the
option names and the corresponding choices, leading to non-sensical docs
such as reported in substrait-io#755. This PR collects the option names and
corresponding choices jointly such that no mix-up can happen. I have
looked at the output that is changed by this script but only
superficially so: my impression is that (1) some options now come in a
different order (because the previous order was determined by the order
of keys in a `dict`, which is implementation-defined) and (2) the
remaining changes come from the intention of this PR, i.e., the
constitute fixes of previously non-sensical output. However, since I am
not intrically familiar with all extension functions, I am not 100%
sure.

Signed-off-by: Ingo Müller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arith divide options are mixed up
2 participants