Skip to content
This repository was archived by the owner on Jul 16, 2022. It is now read-only.

Restore ability to use tempfile kwargs other than dir#40

Merged
untitaker merged 1 commit into
untitaker:masterfrom
GlenWalker:master
Feb 1, 2019
Merged

Restore ability to use tempfile kwargs other than dir#40
untitaker merged 1 commit into
untitaker:masterfrom
GlenWalker:master

Conversation

@GlenWalker

Copy link
Copy Markdown
Contributor

The changes in #38 broke the ability to use prefix, suffix and bufsize arguments when creating the tempfile. This pull request restores the ability, and is intended to fix #39

@GlenWalker

Copy link
Copy Markdown
Contributor Author

I see that the tox stylecheck fails. If you have any interest in this PR I'll fix it up to conform.

@untitaker

Copy link
Copy Markdown
Owner

lgtm. Please just type out the kwargs in the function signature though.

if you could take care of the styling issues that would also be great, yeah.

@untitaker untitaker self-assigned this Jan 31, 2019
@GlenWalker

Copy link
Copy Markdown
Contributor Author

Would you like named parameters corresponding to both tempfile.mkstemp() and io.open(), or just tempfile.mkstemp()? As you mentioned in #38 removing kwargs entirely in favor of explicit options is nicer for API docs, and I tend to agree.

@untitaker

Copy link
Copy Markdown
Owner

So I would like the following params: suffix,prefix,dir,buffering

The changes in untitaker#38 broke the ability to use prefix, suffix and bufsize arguments when creating the tempfile. This commit restores the ability.
@GlenWalker

GlenWalker commented Feb 1, 2019

Copy link
Copy Markdown
Contributor Author

I have updated get_fileobject to take suffix, prefix, dir, **kwargs - buffering is destined for io.open so it gets left in kwargs. The bufsize handling was just an artifact of using tempfile.NamedTemporaryFile which has a bufsize argument (like os.fdopen) argument, while open and io.open both call the argument buffering

@untitaker untitaker left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

good stuff

@untitaker untitaker merged commit 1e7cce5 into untitaker:master Feb 1, 2019
@untitaker

Copy link
Copy Markdown
Owner

released 1.3.0, have fun :)

@GlenWalker

Copy link
Copy Markdown
Contributor Author

Awesome, thanks! This project has saved me from having to figure this all out myself :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of prefix, suffix and bufsize for tempfile broken by #37

2 participants