Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 26, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8c2654
TST / string dtype: add env variable to enable future_string and add …
jorisvandenbossche Apr 27, 2024
4c3042d
Merge remote-tracking branch 'upstream/main' into string-option-enabl…
jorisvandenbossche Jul 24, 2024
3860a11
Merge remote-tracking branch 'upstream/main' into string-option-enabl…
jorisvandenbossche Jul 26, 2024
57b2464
suppress pytest failures when future_string is enabled
jorisvandenbossche Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/unit-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
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: '1'
PANDAS_FUTURE_INFER_STRING: ${{ matrix.pandas_future_infer_string || '0' }}
TEST_ARGS: ${{ matrix.test_args || '' }}
PYTEST_WORKERS: 'auto'
PYTEST_TARGET: ${{ matrix.pytest_target || 'pandas' }}
Expand Down
6 changes: 6 additions & 0 deletions ci/run_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,11 @@ if [[ "$PATTERN" ]]; then
PYTEST_CMD="$PYTEST_CMD -m \"$PATTERN\""
fi

# temporarily let pytest always succeed (many tests are not yet passing in the
# build enabling the future string dtype)
if [[ "$PANDAS_FUTURE_INFER_STRING" == "1" ]]; then
PYTEST_CMD="$PYTEST_CMD || true"
fi
Comment on lines +19 to +23
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 could also add a flag to pytest and then change the exit code from conftest.py (like in https://github.com/yashtodi94/pytest-custom_exit_code/blob/master/pytest_custom_exit_code.py), but since this is only temporary, this seems the easiest short term hack.


echo $PYTEST_CMD
sh -c "$PYTEST_CMD"
2 changes: 1 addition & 1 deletion pandas/core/config_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
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