Skip to content

Add Docs and Examples and helper methods to PhysicalSortExpr - #12589

Merged
alamb merged 1 commit into
apache:mainfrom
alamb:alamb/sort_expr_docs
Sep 23, 2024
Merged

Add Docs and Examples and helper methods to PhysicalSortExpr#12589
alamb merged 1 commit into
apache:mainfrom
alamb:alamb/sort_expr_docs

Conversation

@alamb

@alambalamb commented Sep 23, 2024

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #12446

Closes #.

Rationale for this change

Basically while working on #12562 I got annoyed at the lack of documentation and display implementations as well as how award it was to create PhysicalSortExprs.

What changes are included in this PR?

  1. Added a bunch of docs and display impls and examples

Are these changes tested?

Yes, by CI and doc tests

Are there any user-facing changes?

Better docs, easier to use APIs.

No API changes

@alambalamb added the documentation Improvements or additions to documentation label Sep 23, 2024
@github-actionsgithub-actionsBot added physical-expr Changes to the physical-expr crates core Core DataFusion crate and removed documentation Improvements or additions to documentation labels Sep 23, 2024
nulls_first: true,
},
},
PhysicalSortExpr::new_default(physical_col("string_col", &schema).unwrap())

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This shows what I would like to be able to do


/// Represents Sort operation for a column in a RecordBatch
///
/// Example:

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This looks like a massive example but it renders neatly I think:

Screenshot 2024-09-23 at 8 56 58 AM

@compheadcomphead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm thanks @alamb

@alamb

Copy link
Copy Markdown
ContributorAuthor

Thanks for the quick review @comphead

@alamb
alamb merged commit 04895c4 into apache:mainSep 23, 2024
bgjackma pushed a commit to bgjackma/datafusion that referenced this pull request Sep 25, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coreCore DataFusion cratephysical-exprChanges to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@alamb@comphead