Skip to content

Conversation

@rxxg
Copy link
Contributor

@rxxg rxxg commented Aug 5, 2020

Fixes #35566

@jbrockmendel
Copy link
Member

can you add a test

@rxxg
Copy link
Contributor Author

rxxg commented Aug 6, 2020

Very difficult to add a test since the failure is during the constructor meaning that we have no references to the failed state.
I have pulled something together, but it requires a dependency on psutils and assumes that the test file is on a local disk. Is that acceptable?

@topper-123 topper-123 added IO SAS SAS: read_sas Bug labels Aug 6, 2020
@topper-123 topper-123 modified the milestones: 1.2, 1.1.1 Aug 6, 2020
raise ValueError("unknown SAS format")

if iterator or chunksize:
return reader
Copy link
Contributor

@topper-123 topper-123 Aug 6, 2020

Choose a reason for hiding this comment

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

The problem is potentialy here also, e.g. if the user forgets to wrap his own code in a try/finally.

Could it be an idea to add __enter__ & __exit__ to ReaderBase and rearrange to use with blocks, both here and below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What the user does with the Reader object is not pandas' concern ;)
The important thing is not to leave any created objects in an inconsistent state.

@topper-123
Copy link
Contributor

A dependency on external libs for this is not great IMO. Are there tests for closed files for other file formats, e.g. excel or csv files? Could you try to emulate them?

@rxxg
Copy link
Contributor Author

rxxg commented Aug 6, 2020

OK, let's follow up in the new PR #35587 (sorry, I deleted my repo in the meantime, so need a new PR).

@rxxg rxxg closed this Aug 6, 2020
@simonjayhawkins simonjayhawkins removed this from the 1.1.1 milestone Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug IO SAS SAS: read_sas

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: corrupted SAS datasets retain file handles

4 participants