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

TST / string dtype: add env variable to enable future_string and add test build #58459

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/unit-tests.yml
Expand Up @@ -57,6 +57,10 @@ jobs:
# Also install zh_CN (its encoding is gb2312) but do not activate it.
# It will be temporarily activated during tests with locale.setlocale
extra_loc: "zh_CN"
- name: "Future infer strings"
env_file: actions-311.yaml
pattern: "not slow and not network and not single_cpu"
pandas_future_infer_string: "1"
- name: "Pypy"
env_file: actions-pypy-39.yaml
pattern: "not slow and not network and not single_cpu"
Expand All @@ -75,6 +79,7 @@ jobs:
LANG: ${{ matrix.lang || 'C.UTF-8' }}
LC_ALL: ${{ matrix.lc_all || '' }}
PANDAS_CI: ${{ matrix.pandas_ci || '1' }}
PANDAS_FUTURE_INFER_STRING: ${{ matrix.pandas_future_infer_string || '0' }}
TEST_ARGS: ${{ matrix.test_args || '' }}
PYTEST_WORKERS: ${{ matrix.pytest_workers || 'auto' }}
PYTEST_TARGET: ${{ matrix.pytest_target || 'pandas' }}
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/config_init.py
Expand Up @@ -858,7 +858,7 @@ def register_converter_cb(key: str) -> None:
with cf.config_prefix("future"):
cf.register_option(
"infer_string",
False,
True if os.environ.get("PANDAS_FUTURE_INFER_STRING", "0") == "1" else False,
Copy link
Member

Choose a reason for hiding this comment

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

can we make this something that is explicitly FOR_TESTING_ONLY or something to clarify users shouldn't use it

Copy link
Member Author

Choose a reason for hiding this comment

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

I would personally prefer to keep the name a bit more readable. We did the same for CoW, I don't think we had any problems with it?
I also want to use this for testing in downstream packages, and would encourage others to do this as well (of course, that is still in a context of testing, but I feel that FOR_TESTING_ONLY would indicate it's only for testing for pandas devs)

(in general, there is not really a back compat issue with the env variable, in pandas 3.0 we will just start ignoring it, which is even easier than the explicit pd.options option, which will start raising an error at some point)

"Whether to infer sequence of str objects as pyarrow string "
"dtype, which will be the default in pandas 3.0 "
"(at which point this option will be deprecated).",
Expand Down