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

Doc gen: Attributes to support related_udf, alternative_syntax #13575

Merged
merged 3 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 24 additions & 35 deletions datafusion/doc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ pub struct Documentation {

impl Documentation {
/// Returns a new [`DocumentationBuilder`] with no options set.
pub fn builder() -> DocumentationBuilder {
DocumentationBuilder::new()
pub fn builder(
doc_section: DocSection,
description: impl Into<String>,
syntax_example: impl Into<String>,
) -> DocumentationBuilder {
DocumentationBuilder::new(doc_section, description, syntax_example)
}
}

Expand Down Expand Up @@ -86,29 +90,30 @@ pub struct DocSection {
/// description: None,
/// };
///
/// let documentation = Documentation::builder()
/// .with_doc_section(doc_section)
/// .with_description("Add one to an int32")
/// .with_syntax_example("add_one(2)")
/// let documentation = Documentation::builder(doc_section, "Add one to an int32".to_owned(), "add_one(2)".to_owned())
/// .with_argument("arg_1", "The int32 number to add one to")
/// .build();
/// # }
pub struct DocumentationBuilder {
pub doc_section: Option<DocSection>,
pub description: Option<String>,
pub syntax_example: Option<String>,
pub doc_section: DocSection,
pub description: String,
pub syntax_example: String,
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(
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Thanks @alamb and @Omega359

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

doc_section: DocSection,
description: impl Into<String>,
syntax_example: impl Into<String>,
) -> Self {
Self {
doc_section: None,
description: None,
syntax_example: None,
doc_section,
description: description.into(),
syntax_example: syntax_example.into(),
sql_example: None,
arguments: None,
alternative_syntax: None,
Expand All @@ -117,17 +122,17 @@ impl DocumentationBuilder {
}

pub fn with_doc_section(mut self, doc_section: DocSection) -> Self {
self.doc_section = Some(doc_section);
self.doc_section = doc_section;
self
}

pub fn with_description(mut self, description: impl Into<String>) -> Self {
self.description = Some(description.into());
self.description = description.into();
self
}

pub fn with_syntax_example(mut self, syntax_example: impl Into<String>) -> Self {
self.syntax_example = Some(syntax_example.into());
self.syntax_example = syntax_example.into();
self
}

Expand Down Expand Up @@ -205,30 +210,14 @@ impl DocumentationBuilder {
related_udfs,
} = self;

if doc_section.is_none() {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

panic!("Documentation must have a doc section");
}
if description.is_none() {
panic!("Documentation must have a description");
}
if syntax_example.is_none() {
panic!("Documentation must have a syntax_example");
}

Documentation {
doc_section: doc_section.unwrap(),
description: description.unwrap(),
syntax_example: syntax_example.unwrap(),
doc_section,
description,
syntax_example,
sql_example,
arguments,
alternative_syntax,
related_udfs,
}
}
}

impl Default for DocumentationBuilder {
fn default() -> Self {
Self::new()
}
}
5 changes: 1 addition & 4 deletions datafusion/expr/src/udaf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,7 @@ where
///
/// fn get_doc() -> &'static Documentation {
/// DOCUMENTATION.get_or_init(|| {
/// Documentation::builder()
/// .with_doc_section(DOC_SECTION_AGGREGATE)
/// .with_description("calculates a geometric mean")
/// .with_syntax_example("geo_mean(2.0)")
/// Documentation::builder(DOC_SECTION_AGGREGATE, "calculates a geometric mean", "geo_mean(2.0)")
/// .with_argument("arg1", "The Float64 number for the geometric mean")
/// .build()
/// })
Expand Down
5 changes: 1 addition & 4 deletions datafusion/expr/src/udf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,10 +374,7 @@ pub struct ScalarFunctionArgs<'a> {
///
/// fn get_doc() -> &'static Documentation {
/// DOCUMENTATION.get_or_init(|| {
/// Documentation::builder()
/// .with_doc_section(DOC_SECTION_MATH)
/// .with_description("Add one to an int32")
/// .with_syntax_example("add_one(2)")
/// Documentation::builder(DOC_SECTION_MATH, "Add one to an int32", "add_one(2)")
/// .with_argument("arg1", "The int32 number to add one to")
/// .build()
/// })
Expand Down
5 changes: 1 addition & 4 deletions datafusion/expr/src/udwf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,7 @@ where
///
/// fn get_doc() -> &'static Documentation {
/// DOCUMENTATION.get_or_init(|| {
/// Documentation::builder()
/// .with_doc_section(DOC_SECTION_ANALYTICAL)
/// .with_description("smooths the windows")
/// .with_syntax_example("smooth_it(2)")
/// Documentation::builder(DOC_SECTION_ANALYTICAL, "smooths the windows", "smooth_it(2)")
/// .with_argument("arg1", "The int32 number to smooth by")
/// .build()
/// })
Expand Down
7 changes: 1 addition & 6 deletions datafusion/functions-aggregate/src/approx_distinct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,7 @@ static DOCUMENTATION: OnceLock<Documentation> = OnceLock::new();

fn get_approx_distinct_doc() -> &'static Documentation {
DOCUMENTATION.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_APPROXIMATE)
.with_description(
"Returns the approximate number of distinct input values calculated using the HyperLogLog algorithm.",
)
.with_syntax_example("approx_distinct(expression)")
Documentation::builder(DOC_SECTION_APPROXIMATE, "Returns the approximate number of distinct input values calculated using the HyperLogLog algorithm.", "approx_distinct(expression)")
.with_sql_example(r#"```sql
> SELECT approx_distinct(column_name) FROM table_name;
+-----------------------------------+
Expand Down
9 changes: 4 additions & 5 deletions datafusion/functions-aggregate/src/approx_median.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,11 @@ static DOCUMENTATION: OnceLock<Documentation> = OnceLock::new();

fn get_approx_median_doc() -> &'static Documentation {
DOCUMENTATION.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_APPROXIMATE)
.with_description(
Documentation::builder(
DOC_SECTION_APPROXIMATE,
"Returns the approximate median (50th percentile) of input values. It is an alias of `approx_percentile_cont(x, 0.5)`.",
)
.with_syntax_example("approx_median(expression)")

"approx_median(expression)")
.with_sql_example(r#"```sql
> SELECT approx_median(column_name) FROM table_name;
+-----------------------------------+
Expand Down
8 changes: 3 additions & 5 deletions datafusion/functions-aggregate/src/approx_percentile_cont.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,12 +280,10 @@ static DOCUMENTATION: OnceLock<Documentation> = OnceLock::new();

fn get_approx_percentile_cont_doc() -> &'static Documentation {
DOCUMENTATION.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_APPROXIMATE)
.with_description(
Documentation::builder(
DOC_SECTION_APPROXIMATE,
"Returns the approximate percentile of input values using the t-digest algorithm.",
)
.with_syntax_example("approx_percentile_cont(expression, percentile, centroids)")
"approx_percentile_cont(expression, percentile, centroids)")
.with_sql_example(r#"```sql
> SELECT approx_percentile_cont(column_name, 0.75, 100) FROM table_name;
+-------------------------------------------------+
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,11 @@ static DOCUMENTATION: OnceLock<Documentation> = OnceLock::new();

fn get_approx_percentile_cont_with_weight_doc() -> &'static Documentation {
DOCUMENTATION.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_APPROXIMATE)
.with_description(
Documentation::builder(
DOC_SECTION_APPROXIMATE,
"Returns the weighted approximate percentile of input values using the t-digest algorithm.",
)
.with_syntax_example("approx_percentile_cont_with_weight(expression, weight, percentile)")

"approx_percentile_cont_with_weight(expression, weight, percentile)")
.with_sql_example(r#"```sql
> SELECT approx_percentile_cont_with_weight(column_name, weight_column, 0.90) FROM table_name;
+----------------------------------------------------------------------+
Expand Down
9 changes: 4 additions & 5 deletions datafusion/functions-aggregate/src/array_agg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,11 @@ static DOCUMENTATION: OnceLock<Documentation> = OnceLock::new();

fn get_array_agg_doc() -> &'static Documentation {
DOCUMENTATION.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_GENERAL)
.with_description(
Documentation::builder(
DOC_SECTION_GENERAL,
"Returns an array created from the expression elements. If ordering is required, elements are inserted in the specified order.",
)
.with_syntax_example("array_agg(expression [ORDER BY expression])")

"array_agg(expression [ORDER BY expression])")
.with_sql_example(r#"```sql
> SELECT array_agg(column_name ORDER BY other_column) FROM table_name;
+-----------------------------------------------+
Expand Down
21 changes: 10 additions & 11 deletions datafusion/functions-aggregate/src/average.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,24 +248,23 @@ static DOCUMENTATION: OnceLock<Documentation> = OnceLock::new();

fn get_avg_doc() -> &'static Documentation {
DOCUMENTATION.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_GENERAL)
.with_description(
"Returns the average of numeric values in the specified column.",
)
.with_syntax_example("avg(expression)")
.with_sql_example(
r#"```sql
Documentation::builder(
DOC_SECTION_GENERAL,
"Returns the average of numeric values in the specified column.",
"avg(expression)",
)
.with_sql_example(
r#"```sql
> SELECT avg(column_name) FROM table_name;
+---------------------------+
| avg(column_name) |
+---------------------------+
| 42.75 |
+---------------------------+
```"#,
)
.with_standard_argument("expression", None)
.build()
)
.with_standard_argument("expression", None)
.build()
})
}

Expand Down
41 changes: 21 additions & 20 deletions datafusion/functions-aggregate/src/bit_and_or_xor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,40 +139,41 @@ static BIT_AND_DOC: OnceLock<Documentation> = OnceLock::new();

fn get_bit_and_doc() -> &'static Documentation {
BIT_AND_DOC.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_GENERAL)
.with_description("Computes the bitwise AND of all non-null input values.")
.with_syntax_example("bit_and(expression)")
.with_standard_argument("expression", Some("Integer"))
.build()
Documentation::builder(
DOC_SECTION_GENERAL,
"Computes the bitwise AND of all non-null input values.",
"bit_and(expression)",
)
.with_standard_argument("expression", Some("Integer"))
.build()
})
}

static BIT_OR_DOC: OnceLock<Documentation> = OnceLock::new();

fn get_bit_or_doc() -> &'static Documentation {
BIT_OR_DOC.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_GENERAL)
.with_description("Computes the bitwise OR of all non-null input values.")
.with_syntax_example("bit_or(expression)")
.with_standard_argument("expression", Some("Integer"))
.build()
Documentation::builder(
DOC_SECTION_GENERAL,
"Computes the bitwise OR of all non-null input values.",
"bit_or(expression)",
)
.with_standard_argument("expression", Some("Integer"))
.build()
})
}

static BIT_XOR_DOC: OnceLock<Documentation> = OnceLock::new();

fn get_bit_xor_doc() -> &'static Documentation {
BIT_XOR_DOC.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_GENERAL)
.with_description(
"Computes the bitwise exclusive OR of all non-null input values.",
)
.with_syntax_example("bit_xor(expression)")
.with_standard_argument("expression", Some("Integer"))
.build()
Documentation::builder(
DOC_SECTION_GENERAL,
"Computes the bitwise exclusive OR of all non-null input values.",
"bit_xor(expression)",
)
.with_standard_argument("expression", Some("Integer"))
.build()
})
}

Expand Down
42 changes: 20 additions & 22 deletions datafusion/functions-aggregate/src/bool_and_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,24 +186,23 @@ static DOCUMENTATION: OnceLock<Documentation> = OnceLock::new();

fn get_bool_and_doc() -> &'static Documentation {
DOCUMENTATION.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_GENERAL)
.with_description(
"Returns true if all non-null input values are true, otherwise false.",
)
.with_syntax_example("bool_and(expression)")
.with_sql_example(
r#"```sql
Documentation::builder(
DOC_SECTION_GENERAL,
"Returns true if all non-null input values are true, otherwise false.",
"bool_and(expression)",
)
.with_sql_example(
r#"```sql
> SELECT bool_and(column_name) FROM table_name;
+----------------------------+
| bool_and(column_name) |
+----------------------------+
| true |
+----------------------------+
```"#,
)
.with_standard_argument("expression", None)
.build()
)
.with_standard_argument("expression", None)
.build()
})
}

Expand Down Expand Up @@ -334,24 +333,23 @@ impl AggregateUDFImpl for BoolOr {

fn get_bool_or_doc() -> &'static Documentation {
DOCUMENTATION.get_or_init(|| {
Documentation::builder()
.with_doc_section(DOC_SECTION_GENERAL)
.with_description(
"Returns true if any non-null input value is true, otherwise false.",
)
.with_syntax_example("bool_or(expression)")
.with_sql_example(
r#"```sql
Documentation::builder(
DOC_SECTION_GENERAL,
"Returns true if any non-null input value is true, otherwise false.",
"bool_or(expression)",
)
.with_sql_example(
r#"```sql
> SELECT bool_or(column_name) FROM table_name;
+----------------------------+
| bool_or(column_name) |
+----------------------------+
| true |
+----------------------------+
```"#,
)
.with_standard_argument("expression", None)
.build()
)
.with_standard_argument("expression", None)
.build()
})
}

Expand Down
Loading