Doc gen: Attributes to support related_udf, alternative_syntax
#13575
Conversation
|
|
| name = "trim_str", | ||
| description = r"String expression to trim from the beginning of the input string. Can be a constant, column, or function, and any combination of arithmetic operators. _Default is whitespace characters._" | ||
| ), | ||
| alternative_syntax = "trim(LEADING trim_str FROM str)", |
There was a problem hiding this comment.
@Omega359 alternative_syntax and related_udf support
| pub sql_example: Option<String>, | ||
| pub arguments: Option<Vec<(String, String)>>, | ||
| pub alternative_syntax: Option<Vec<String>>, | ||
| pub related_udfs: Option<Vec<String>>, | ||
| } | ||
|
|
||
| impl DocumentationBuilder { | ||
| pub fn new() -> Self { | ||
| pub fn new( |
There was a problem hiding this comment.
for anyone following along these settings are required, otherwise the builder will error (or now panic)
So I think the idea here is that the compiler can now verify the required fields which seems like an improvement to me, even though it is an API change
cc @Omega359
There was a problem hiding this comment.
Gain one thing, loose another. Those fields now no longer have obvious names as they did previously if you are looking at them outside of an IDE. It's ok but it's obviously not the choice I made.
There was a problem hiding this comment.
Thats' correct. But another thing to mention the builder API mostly gonna be used internally within documentation generator crate so the user has to deal with attributes rather than API itself.
The following PR I'm intending to replace API approach to attributes where it is possible to, probably going crate by crate.
There was a problem hiding this comment.
Thats' correct. But another thing to mention the builder API mostly gonna be used internally within documentation generator crate so the user has to deal with attributes rather than API itself.
Ah, thanks for mentioning that as I had forgotten that.
| @@ -205,30 +210,14 @@ impl DocumentationBuilder { | |||
| related_udfs, | |||
| } = self; | |||
|
|
|||
| if doc_section.is_none() { | |||
| .with_description("Casts a value to a specific Arrow data type.") | ||
| .with_syntax_example("arrow_cast(expression, datatype)") | ||
| Documentation::builder( | ||
| DOC_SECTION_OTHER, |
There was a problem hiding this comment.
it is somewhat unfortunate that we lose the builder syntax but I think it is worth it
There was a problem hiding this comment.
thanks @alamb. imho, it is regular builder pattern for the object construction which has both mandatory fields(in constructor) and optional fields(set by builder).
Which issue does this PR close?
Followup on #12822
Closes #13553
Rationale for this change
related_udf,alternative_syntaxLTRIMto attrubute based doc gen instead of APIWhat changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?