BUG: read_hdf modifying passed columns list (GH7212) #10055
Conversation
| @@ -4022,7 +4022,11 @@ def read(self, where=None, columns=None, **kwargs): | |||
| df = concat(frames, axis=1, verify_integrity=False).consolidate() | |||
|
|
|||
| # apply the selection filters & axis orderings | |||
| df = self.process_axes(df, columns=columns) | |||
| if type(columns) is list: | |||
There was a problem hiding this comment.
Checking for specific types is generally a bad idea -- what if, for example, a user passes in a list subclass, or another sequence type?
A better approach is to cast any provided columns to a list:
if columns is not None:
columns = list(columns)However, given that the mutation happens in the process_axes method, it would be a better idea to put this there.
|
I know it seems somewhat contrived, but we do try to add tests even for things like this. |
jreback
changed the title
|
@shoyer thanks for the feedback! much better solution. i'll get a test together as well. |
|
@ajamian can you add a test for this? |
|
@jreback absolutely, should be getting to it this weekend. |
|
@ajamian one last thing -- can you add a note to the release notes for 0.17? otherwise this looks good to me! |
|
yep lgtm as well - pls add a comment on the test with the issue number |
|
I put the note into doc/source/whatsnew/v0.17.0.txt, I think this is where it belongs...and comment added to test. Thanks guys - let me know if you have any more comments. |
BUG: read_hdf modifying passed columns list (GH7212)
|
Thanks! |
|
thanks Tom! |
Closes #7212. If this object is a list, make a copy and then pass to process_axes - otherwise, just pass the object directly (in this case it is most likely none).
A test for this behavior seems complex and somewhat contrived when compared with what exists in test_pytables.py, so I didn't add one --- let me know any thoughts on this.