Skip to content

Conversation

@akiross
Copy link

@akiross akiross commented Sep 27, 2016

I would like to propose to introduce a new method, download_to, that uses a file-like object where data is written. This allows to download to memory as well as to temporary files, without having the user to know the file path.

This may be useful for bots where the file does not need to be stored, for example for describe-the-image bots or bots that modify the image on-the-fly and send it back, without having to store it.

The `download_to` method uses the provided file-object to store the data. This allows to use memory buffers as well as temporary files without having to pass by a file name.
@jsmnbom
Copy link
Member

jsmnbom commented Sep 29, 2016

I definitely think we should have a method like this, I'm just not sure about the naming. To me download_to sounds a lot more like you're trying to download to a file, not just a file object.
We could also just make download accept either a string (the path) or a file-like-object?
I'd like to hear the other developers feelings about this though...

@akiross
Copy link
Author

akiross commented Sep 29, 2016

First, let me apologize because I don't understand where the code issue is. The code is running fine on my test bot, and I am not expert at all with travis and coverall. Any suggestion is welcome, so I can try to fix.

Second, yes: I thought about the naming as well. My first idea is to make download() accepting a file object in addition to the path string, but this implied checking for the type (which can be as simple as trying to write() to it and, if failing, assume it's a string and proceed as it is now), but I don't love the parameters where the type can vary significantly.

So I thought about passing a new, optional parameter for the file object, but this means that the current parameter has to be made optional as well, leading to have an API where two parameters are both optional, but one is required, and their use is mutually exclusive. Not a really clear choice, I believe.

So I thought about a separated method. Yes, download_to can definitely be improved; maybe... download_into(writable)? But I cannot come up with a name that is clear and concise at the same time (download_into_file_like_object()).

@jh0ker
Copy link
Member

jh0ker commented Nov 8, 2016

Closing in favour of #459

@jh0ker jh0ker closed this Nov 8, 2016
@github-actions github-actions bot locked and limited conversation to collaborators Aug 24, 2020
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.

3 participants