Enable moving files to folder IDs as well as names #48
Conversation
… using the folder name
| I was unable to see what Better Code Hub complained about this PR as I do not have the required permissions. Please advise on any changes that must be made. Thanks! |
| Hi! I'm so sorry I hadn't gotten to this, I must have missed the e-mail notification about it (and I've been busy with a new job so I hadn't been checking the repo regularly). I'll try to take a look this week, and thanks so much for your contribution! |
aiguofer left a comment
aiguofer left a comment There was a problem hiding this comment.
Thanks for your contribution!! I definitely agree that adding the ability to use the folder ID is useful, and this PR seems to address that. I made a couple suggestions and would be happy to merge after those are addressed.
| return parent | ||
| | ||
| def move_file(self, file_id, path, create=False): | ||
| def move_file(self, file_id, path, id_string, create=False): |
There was a problem hiding this comment.
id_string here is the folder id, right? if so, I think folder_id would be more descriptive (id_string indicates it's an ID and a string, but it's not clear an id for what).
Since folder_id becomes an alternative to path, we should make both parameters optional with a default of None and a check that at least one of them is provided.
| return self.client.list_permissions(self.spread.id) | ||
| | ||
| def move(self, path="/", create=True): | ||
| def move(self, path="/", id_string=None, create=True): |
There was a problem hiding this comment.
same as above, should be named folder_id
| Hi, I found this PR and this functionality would be quite useful to me. I made the changes that you suggested but I'm not really sure how to submit them to this existing PR but I also didn't want to just create a new one and add clutter. What would be the best/preferred way to handle this is? |
3 participants
Simple change which fixed an issue I was having (and which is currently mentioned in issue #46 ) where a folder cannot be found by name when using the move command but whose ID can be located with the gspread_pandas client or by examining the URL. This enables moving a file to a folder (using either client.move_file() or spread.move()) using its id, rather than its name. This is done with the id_string argument. When not explicitly using the id_string argument, it behavies identically to previous behaviour.
This is my first time contributing to open source, please forgive any mistakes in the process. Hope this helps!