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

docs(python): improve docstring of (Expr|Series).map_elements #16079

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

KDruzhkin
Copy link
Contributor

Mention:
col("col_name").struct.field("field_name").native_method().

(Follow-up on #15887).

Comment on lines 4610 to 4613
- For mapping inner elements of structs, consider:
`pl.col("col_name").struct.field("field_name").sqrt()`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice

just thinking out loud - this extracts just that field, so it feels a bit different from the other items on the list. 🤔 thinking if there's a better way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 think, the design space here is different from e.g. pandas DataFrames because of immutability. Instead of updating the frame in-place, we (at least conceptually) create a copy of the original frame with some parts modified.

When immutability meets deep nesting... I heard, there are exotic solutions, e.g. lenses.

Copy link

codecov bot commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.40%. Comparing base (8bc1019) to head (9be3e60).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #16079   +/-   ##
=======================================
  Coverage   81.40%   81.40%           
=======================================
  Files        1409     1409           
  Lines      184519   184519           
  Branches     2961     2961           
=======================================
+ Hits       150206   150209    +3     
+ Misses      33798    33795    -3     
  Partials      515      515           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KDruzhkin KDruzhkin changed the title improve docstring of (Expr|Series).map_elements docs(python): improve docstring of (Expr|Series).map_elements May 22, 2024
@github-actions github-actions bot added documentation Improvements or additions to documentation python Related to Python Polars labels May 22, 2024
@MarcoGorelli
Copy link
Collaborator

struct.with_fields has been added, fancy updating to use that?

@KDruzhkin
Copy link
Contributor Author

KDruzhkin commented May 22, 2024

struct.with_fields has been added, fancy updating to use that?

Thought of it. But the names are not local to the struct, and this expression is too heavy for a docstring:

pl.col("col_name").struct.with_fields(
    pl.col("col_name").struct.field("field_name").sqrt()
)

@KDruzhkin
Copy link
Contributor Author

#16395

@KDruzhkin
Copy link
Contributor Author

struct.with_fields has been added, fancy updating to use that?

Done. Had to use blockquotes >>> to fit the examples.

Copy link
Collaborator

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

thanks! just left a comment about the reference to with_columns / with_fields, but the content looks good, definitely good to hammer home the message here IMO, this is probably the #1 most misused Polars method

`pl.col("col_name").struct.field("field_name").sqrt()`.

If you want to replace the original column or field,
consider `.with_columns` and `.with_fields`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

could these be links? something like

:meth:`DataFrame.with_columns`

and something similar for with_fields

I can't remember the exact syntax, you may need to build the documentation locally to check it works

Copy link
Contributor Author

Choose a reason for hiding this comment

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

links

Comment on lines 4619 to 4623
>>> new_expr = pl.col("col_name").sqrt()
>>> df.with_columns(new_expr)

>>> new_expr = pl.col("col_name").struct.field("field_name").sqrt()
>>> df.with_columns(pl.col("col_name").struct.with_fields(new_expr))
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is causing a doctest failure - I'd suggest, either remove it (you're already linking to these methods above) or use a codeblock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants