Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Synchronize io/stata with pandas master #202

Merged
merged 6 commits into from
Aug 22, 2022

Conversation

bashtage
Copy link
Contributor

Sychronize and remvoe classes not part of the public API

  • Tests added: Please use assert_type() to assert the type of any return value

@bashtage
Copy link
Contributor Author

No tests yet, but can do some here

columns: list[HashableT] | None = ...,
order_categoricals: bool = ...,
chunksize: int | None = ...,
*,
Copy link
Member

Choose a reason for hiding this comment

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

so the second and third overload only different in whether the arguments are keyword-only or can also be provided as positional arguments?

If pandas would have deprecated positional arguments, I would remove the positional overloads - but they are not yet deprecated. (Will try to open a PR for that later today at pandas.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. They are needed to handle the cases where iterator=True either using a keyword argument or only positional.

Copy link
Member

Choose a reason for hiding this comment

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

I opened pandas-dev/pandas#48128 to make most arguments keyword-only

Copy link
Member

Choose a reason for hiding this comment

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

using positional keywords will now be deprecated in 1.5 (except for the first non-self argument)

Sychronize and remvoe classes not part of the public API
@bashtage
Copy link
Contributor Author

Test added. I took the CM from pandas. It could be made available somewhere else, or we could stub it in even though it is not public.

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

I realize that you copied this from pandas source, but for the stubs, we can make things narrower.

pandas-stubs/core/frame.pyi Outdated Show resolved Hide resolved
path: FilePathOrBuffer,
convert_dates: dict | None = ...,
path: FilePath | WriteBuffer[bytes],
convert_dates: dict[Hashable, str] | None = ...,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the code, only values for str are [ "tc", "%tc", "td", "%td", "tw", "%tw", "tm", "%tm", "tq", "%tq", "th", "%th", "ty", "%ty", ]

pandas-stubs/core/frame.pyi Show resolved Hide resolved
tests/test_io.py Outdated Show resolved Hide resolved
Add literals for limited value inputs
pandas-stubs/io/stata.pyi Show resolved Hide resolved
pandas-stubs/io/stata.pyi Show resolved Hide resolved
) -> None: ...
def generate_table(self) -> tuple[dict[str, tuple[int, int]], DataFrame]: ...
def generate_blob(self, gso_table: dict[str, tuple[int, int]]) -> bytes: ...

class StataWriter117(StataWriter):
Copy link
Collaborator

Choose a reason for hiding this comment

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

StataWriter117 is not public, so we can delete

convert_strl: Sequence[Hashable] | None = ...,
compression: CompressionOptions = ...,
storage_options: StorageOptions = ...,
*,
value_labels: dict[Hashable, dict[float, str]] | None = ...,
value_labels: dict[HashableT, dict[float, str]] | None = ...,
) -> None: ...

class StataWriterUTF8(StataWriter117):
Copy link
Collaborator

Choose a reason for hiding this comment

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

StataWriterUTF8 is not public, so we can delete

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

thanks @bashtage

@Dr-Irv Dr-Irv merged commit a45600d into pandas-dev:main Aug 22, 2022
@bashtage bashtage deleted the io-stata branch August 23, 2022 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants