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

Refactor resource requests out of Expressions #528

Merged
merged 9 commits into from Feb 1, 2023

Conversation

jaychia
Copy link
Contributor

@jaychia jaychia commented Jan 31, 2023

  1. Expressions no longer have an associated .resource_request() function, only LogicalPlans.
  2. @udf(...) no longer allows users to specify resource requests in the UDF
  3. The only DataFrame method that allows for custom resource requests is .with_column("x", expr, resource_request=...)

@jaychia jaychia force-pushed the jay/refactor-resource-requests branch 3 times, most recently from 553254c to 3380fe3 Compare February 1, 2023 00:28
@jaychia jaychia changed the title Refactor resource requests to use context manager Refactor resource requests out of Expressions Feb 1, 2023
@jaychia jaychia force-pushed the jay/refactor-resource-requests branch from 35ed519 to 3abc8e2 Compare February 1, 2023 06:59
@codecov
Copy link

codecov bot commented Feb 1, 2023

Codecov Report

Merging #528 (e401954) into main (3c99fbb) will decrease coverage by 0.14%.
The diff coverage is 94.28%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #528      +/-   ##
==========================================
- Coverage   72.92%   72.78%   -0.14%     
==========================================
  Files          95       95              
  Lines        7638     7588      -50     
==========================================
- Hits         5570     5523      -47     
+ Misses       2068     2065       -3     
Impacted Files Coverage Δ
daft/expressions.py 93.17% <ø> (-0.17%) ⬇️
daft/logical/map_partition_ops.py 96.66% <ø> (-0.40%) ⬇️
daft/runners/shuffle_ops.py 93.93% <ø> (-0.18%) ⬇️
daft/udf.py 80.00% <50.00%> (-2.09%) ⬇️
daft/dataframe/dataframe.py 89.88% <100.00%> (+0.04%) ⬆️
daft/execution/logical_op_runners.py 96.44% <100.00%> (ø)
daft/logical/logical_plan.py 79.64% <100.00%> (-0.85%) ⬇️
daft/logical/optimizer.py 95.98% <100.00%> (+0.01%) ⬆️
daft/resource_request.py 95.23% <100.00%> (ø)
daft/runners/pyrunner.py 96.92% <100.00%> (ø)
... and 3 more

@jaychia jaychia force-pushed the jay/refactor-resource-requests branch from 7e1cdd8 to ecedcf0 Compare February 1, 2023 18:01
@jaychia jaychia merged commit 4aaddf8 into main Feb 1, 2023
@jaychia jaychia deleted the jay/refactor-resource-requests branch February 1, 2023 20:02
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.

None yet

1 participant