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

Update arrow module docs #1840

Merged
merged 1 commit into from
Jun 10, 2022
Merged

Update arrow module docs #1840

merged 1 commit into from
Jun 10, 2022

Conversation

tustvold
Copy link
Contributor

@tustvold tustvold commented Jun 10, 2022

The module level documentation for arrow whilst accurate I felt could be improved. In particular:

  • Greater focus on how to use the crate and less on what the crate consists of
  • For those not already familiar with arrow, it can be quite hard to follow (Best way to convert arrow to Rust native type #1760)
  • Highlight the statically typed APIs and how to use them to write generic code
  • Document newer APIs for interacting with arrays, e.g. iterators, slices, etc...
  • Less focus on lower-level advanced APIs interacting with buffers, etc...
  • Highlight safety guarantees
  • Call out DataFusion

@github-actions github-actions bot added the arrow Changes to the arrow crate label Jun 10, 2022
@@ -15,114 +15,215 @@
// specific language governing permissions and limitations
// under the License.

//! A native Rust implementation of [Apache Arrow](https://arrow.apache.org), a cross-language
//! A complete, safe, native Rust implementation of [Apache Arrow](https://arrow.apache.org), a cross-language
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing like a good bit of marketeering 😆

//! [the Arrow data layout specification](https://arrow.apache.org/docs/format/Columnar.html).
//! For example, the type `Int16Array` represents an Apache
//! Arrow array of 16-bit integers.
//! # Downcasting an Array
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 feel it is important to highlight this from the outset, as it can be unclear for a new user given an Array how to actually do something with it 😆

//! # use arrow::array::ListArray;
//! # use arrow::datatypes::Int32Type;
//! #
//! Int32Array::from(vec![1, 2]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These new APIs are 👌

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree -- this is very nice

//! let array = Arc::new(Int32Array::from_iter([1, 2, 3])) as ArrayRef;
//!
//! // Slice with offset 1 and length 2
//! let sliced = array.slice(1, 2);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is perhaps a little bit unfortunate that this returns ArrayRef even when called on a concrete type, but then again I'm not sure of many use-cases for slicing concretely type arrays 😆

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

I love it -- thank you @tustvold

While I am sure we can continue to improve the docs, this is a nice step forward. Merging this one in to get into arrow 16.

//! # use arrow::array::ListArray;
//! # use arrow::datatypes::Int32Type;
//! #
//! Int32Array::from(vec![1, 2]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree -- this is very nice

//!
//! ## Field, Schema and RecordBatch
//! It is often the case that code wishes to handle any type of array, without necessarily knowing
Copy link
Contributor

Choose a reason for hiding this comment

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

thank you for this -- it is a good contribution

//! # Safety and Security
//!
//! Like many crates, this crate makes use of unsafe where prudent. However, it endeavours to be
//! sound. Specifically, **it should not be possible to trigger undefined behaviour using safe APIs.**
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@alamb alamb merged commit a57ffa2 into apache:master Jun 10, 2022
@alamb alamb added the documentation Improvements or additions to documentation label Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants