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

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

Conversation

kellyfelkins
Copy link
Contributor

The prior description of DataFrame.arrange_with appears to be missing the word "not".

With less confidence, I thought the phrasing could be improved. This is less important than adding "not".

I'm offering this because one of the things that contributes to my experience with Elixir is the wonderful, quality documentation.

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 josevalim merged commit 132ed1f into elixir-explorer:main Dec 6, 2023
3 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants