CLN/FIX/PERF: Don't buffer entire Stata file into memory
State: closed

Fixes #48700
Closes #48922 (supersedes it)
Refs pandas-dev/pandas#9245
Refs pandas-dev/pandas#37639
Regressed in 6d1541e1782a7b94797d5432922e64a97934cfa4

- [x] closes #48700
- [x] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
  - [x] The existing tests for e.g. roundtripping zstandard-compressed Stata files test this code path.
  - [x] Added a test that checks e.g. a fp or BytesIO passed in is the same object the reader reads. 
  - [x] Added a test that ensures a warning is raised if StataReader isn't being used as a context manager. 
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [x] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [x] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.

---

This PR is a reworking of my earlier #48922, following @bashtage's suggestions. Unfortunately I couldn't quite make it fit in about 15 lines.

Compared to that PR, StataReader required some refactoring to have it behave as it has done before (see the discussion in the other PR) while also making it possible for it to raise a warning when data is being read without a `with` block being in use. (This does not take into account an user having code that doesn't use `with`, but that calls `.close()` by hand.)

The main refactoring is that all of the internal state of the reader object is now `_private`, and public bits are accessed via properties; this allows us to make sure the data is actually read when requested, even without reading happening in `__init__`. (Accessing those properties when the StataReader hasn't been `__enter__`ed will thus also raise a ResourceWarning.) As a bonus, the properties are now well-typed. Previously, e.g. `reader.time_stamp` had to rely on inference...

While renaming those properties, I noticed there was a bunch of repeated `struct.unpack(...read...)[0]` code that would have been `black` reformatted into very unwieldy lines following the addition of the underscores, so I decided to refactor the repeated code into smaller reading utility functions.

Finally, there's basically the same fix I made in #48922 – if the underlying stream is seekable, we can use it directly.

I also added a commit that removes the implicit automatic closing of the underlying stream, since it shouldn't be necessary anymore. If that should be removed from this PR, let me know.

Labels:
name: IO Stata
description: read_stata, to_stata

Comments:
Could I ask @bashtage to review this? I'll take care of the handful of test failures once there's some reviews in – they seem to be related to the fact that we sometimes get the actual leak ResourceWarning and not the one we raise by hand...
@bashtage Could you give this another look? I rebased to fix the changelog conflict.
Rebased once again. @bashtage, could you take a look so we could get this in? 😄 Thanks!
@bashtage Rebased. Could you take another look, please?
@bashtage Sorry to ping you again, but could we move this forward? 😅 
@mroeschke Sorry, how do we move forward with this? Is there something I should do? I'd really quite like to get this merged 😅 
There are a few deprecations from 1.x left over that need to be removed before merging this PR since it introduces a new deprecation 
@mroeschke Rebased. Using `find_stack_level()` now. Is there any chance of getting this into 2.0.0? To be frank, it's a bit frustrating to have had this PR open since October...
Thanks for rebasing. Looks like there's some failing tests, but once those are addressed this should be good to get into 2.0

Sorry for the frustration. We have a very limited amount of reviewers and PRs that are new or consistently rebased tend to get the most attention.
> Thanks for rebasing. Looks like there's some failing tests, but once those are addressed this should be good to get into 2.0

now that the rc is cut, this should target 2.1? backports to 2.0.x should now ideally only be regressions/bugs reported during the rc period?
I was thinking since this PR had been ready & forgotten for a bit while we were cleaning up 1.x deprecations that it would be okay to have this in the official 2.0 release. Additionally, it includes a deprecation that would be nice to show to users in 2.0 rather than 2.1
@mroeschke Tests seem to be mostly green and failures... seem to be spurious and not related to Stata? 
> I was thinking since this PR had been ready & forgotten for a bit while we were cleaning up 1.x deprecations that it would be okay to have this in the official 2.0 release. Additionally, it includes a deprecation that would be nice to show to users in 2.0 rather than 2.1

since this is isolated to stata functionality then no objection if you want to target 2.0
I agree that it is ready and would be good to have in 2.0.
@mroeschke Do you still have any remaining issues that need to be addressed?  If not, could you change to approval?
Yeah I don't think this was addressed https://github.com/pandas-dev/pandas/pull/49228#discussion_r1051447411

Could you add a commit with these changes?

```
diff --git a/doc/source/whatsnew/v2.0.0.rst b/doc/source/whatsnew/v2.0.0.rst
index 0445465507..987b408b77 100644
--- a/doc/source/whatsnew/v2.0.0.rst
+++ b/doc/source/whatsnew/v2.0.0.rst
@@ -855,6 +855,7 @@ Deprecations
 - Deprecated :meth:`Series.backfill` in favor of :meth:`Series.bfill` (:issue:`33396`)
 - Deprecated :meth:`DataFrame.pad` in favor of :meth:`DataFrame.ffill` (:issue:`33396`)
 - Deprecated :meth:`DataFrame.backfill` in favor of :meth:`DataFrame.bfill` (:issue:`33396`)
+- Deprecated :meth:`~pandas.io.stata.StataReader.close`. Use :class:`~pandas.io.stata.StataReader` as a context manager instead (:issue:`49228`)

 .. ---------------------------------------------------------------------------
 .. _whatsnew_200.prior_deprecations:
diff --git a/pandas/io/stata.py b/pandas/io/stata.py
index b3e7b186cb..4bf69f167f 100644
--- a/pandas/io/stata.py
+++ b/pandas/io/stata.py
@@ -1233,7 +1233,7 @@ class StataReader(StataParser, abc.Iterator):
             "will be removed in a future version without notice. "
             "Using StataReader as a context manager is the only supported method.",
             FutureWarning,
-            stacklevel=2,
+            stacklevel=find_stack_level(),
         )
         if self._close_file:
             self._close_file()
```
Thanks for your patience and effort @akx. This will make it into 2.0 when it's released in a few weeks 
@mroeschke Thank you and @bashtage for seeing this through :) Have a great weekend! 🥂 
Thanks for noticing this issue and working through the concerns.

On Fri, Feb 24, 2023, 19:21 Aarni Koskela ***@***.***> wrote:

> @mroeschke <https://github.com/mroeschke> Thank you and @bashtage
> <https://github.com/bashtage> for seeing this through :) Have a great
> weekend! 🥂
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/49228#issuecomment-1444306119>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABKTSRIAZABIEGQHD7LSM73WZEC25ANCNFSM6AAAAAARLEQOTY>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>