Skip to content

Do not persist data to memory by default when creating tables#245

Merged
charlesbluca merged 5 commits intodask-contrib:mainfrom
jdye64:default_persist_false
Oct 12, 2021
Merged

Do not persist data to memory by default when creating tables#245
charlesbluca merged 5 commits intodask-contrib:mainfrom
jdye64:default_persist_false

Conversation

@jdye64
Copy link
Collaborator

@jdye64 jdye64 commented Oct 5, 2021

Currently data is persisted into memory when create_table is invoked. Instead this PR will set the default for persist to False which will prevent that from happening.

This Closes: #218

charlesbluca and others added 2 commits September 24, 2021 11:43
@charlesbluca
Add GPU tests, pytest gpu marker (#1)
efa492f 
* Add dask-cudf tests with GPU marker

* Change --gpu option to --rungpu

* xfail GPU table creation from memory
@jdye64
Do not persist data to memory by default when creating tables
bdd849d
@charlesbluca
Copy link
Collaborator

charlesbluca commented Oct 5, 2021

Now that #240 is merged, you should revert efa492f, as it retains some bugs from the initial setup of gpuCI

ayushdg
ayushdg reviewed Oct 12, 2021
View reviewed changes
Copy link
Collaborator

@ayushdg ayushdg left a comment

Choose a reason for hiding this comment

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

Could you update the doctstring in the params section here as well:

dask-sql/dask_sql/context.py

Line 188 in 475ee7d

persist (:obj:`bool`): Only used when passing a string into the ``input`` parameter.

@jdye64
Copy link
Collaborator Author

jdye64 commented Oct 12, 2021

Could you update the doctstring in the params section here as well:

dask-sql/dask_sql/context.py

Line 188 in 475ee7d

persist (:obj:`bool`): Only used when passing a string into the ``input`` parameter.

Yep, just flipped around the values from False -> True and off->on since the rest of the doc string is still valid.

@jdye64
Copy link
Collaborator Author

jdye64 commented Oct 12, 2021

rerun tests

charlesbluca
charlesbluca approved these changes Oct 12, 2021
View reviewed changes
Copy link
Collaborator

@charlesbluca charlesbluca left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jdye64! Will merge in once tests finish up

@charlesbluca charlesbluca merged commit 8e3925c into dask-contrib:main Oct 12, 2021
@charlesbluca charlesbluca mentioned this pull request Oct 14, 2021
Closed
@jakirkham jakirkham mentioned this pull request Nov 3, 2021
Closed
@jdye64 jdye64 deleted the default_persist_false branch December 21, 2021 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ayushdg ayushdg ayushdg left review comments

@charlesbluca charlesbluca charlesbluca approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consider persist=False as default when creating table from DataFrame

3 participants

@jdye64 @charlesbluca @ayushdg