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.

Sign up for GitHub

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

Hopefully improves the description of DataFrame.arrange_with() #760

Merged
merged 2 commits into from
Dec 6, 2023
Merged

Hopefully improves the description of DataFrame.arrange_with() #760

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e872a68
Hopefully improve description of DataFrame.arrange_with()
kellyfelkins Dec 6, 2023
954f2bf
Update lib/explorer/data_frame.ex
josevalim Dec 6, 2023
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
5 changes: 2 additions & 3 deletions lib/explorer/data_frame.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3150,9 +3150,8 @@ defmodule Explorer.DataFrame do
@doc """
Arranges/sorts rows by columns using a callback function.

The callback receives a lazy dataframe. A lazy dataframe does
hold any values, instead it stores all operations in order to
execute all sorting performantly.
The callback receives a lazy dataframe. A lazy dataframe does not
hold values; instead it stores operations for efficient sorting.
Copy link
Member

Choose a reason for hiding this comment

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

TIL!

I think using ; instead increases the barrier for contribution though... as we are going into advanced english. Could we use something else? "rather"? remove "instead" altogether? Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

If both ";" and "," trip folks up for one reason or another, here's an alternative:

The callback receives a lazy dataframe which stores operations instead of values for efficient sorting.

And this is off topic, but we (I) might want to consider writing a guide on lazy frames. It's a somewhat complex topic. If you come from Pandas there's no equivalent AFAIK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL!

I think using ; instead increases the barrier for contribution though... as we are going into advanced english. Could we use something else? "rather"? remove "instead" altogether? Thoughts?

I think the semicolon is correct, but I normally use a dash or emdash....they seem to make more sense to me.

A lazy dataframe does not hold values–instead it stores operations for efficient sorting.

But I like Billy's suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If both ";" and "," trip folks up for one reason or another, here's an alternative:

The callback receives a lazy dataframe which stores operations instead of values for efficient sorting.

I think this is better. But...

I feel like this sentence is carrying too much and not enough at the same time.

  • it's saying that the callback receives a lazy dataframe (good)
  • then explains the motivation for lazy dataframes (efficient operations - in this case, sorting)
  • but doesn't say what should be returned from the callback...

How about:

The callback receives a Lazy DataFrame and returns a list of columns with optional sort directions.

And this is off topic, but we (I) might want to consider writing a guide on lazy frames. It's a somewhat complex topic. If you come from Pandas there's no equivalent AFAIK.

And a 👍 on this too.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me, and we can improve the docs for Explorer.Backends.LazyFrame!

josevalim marked this conversation as resolved.
Show resolved Hide resolved

This is a callback version of `arrange/2`.

Expand Down
Loading