Skip to content

TST: Fix locale test that fails in GitHub actions #29852

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

Closed
wants to merge 2 commits into from
Closed

TST: Fix locale test that fails in GitHub actions #29852

wants to merge 2 commits into from

Conversation

datapythonista
Copy link
Member

xref #29715

Looks like in GitHub actions this test is receiving an empty string for the environment variable LOCALE_OVERRIDE instead of None. Not sure why, but I guess they should be equivalent, and we don't want to fail when an empty string is provided.

The change also makes the test return a descriptive error message if an unexpected value is set in that environment variable.

@datapythonista datapythonista added Testing pandas testing functions or related to the test suite Error Reporting Incorrect or improved errors from pandas labels Nov 26, 2019
"Unknown format for LOCALE_OVERRIDE, "
"expected country_VARIANT.ENCODING "
f"(e.g. en_US.UTF-8), found {locale_override!r}"
)
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused why we're raising an error in a test instead of just skipping for example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. This test is already failing if the locale got an incorrect format. I just wanted that when it fails, it provides a useful error, so we can fix the locale in the CI. An assert could make more sense, but raising made the code simpler.

I'm confused myself about how the localization works. Not even sure if it works, looking at https://github.com/pandas-dev/pandas/blob/master/ci/run_tests.sh#L13. And I have the feeling that we should simply test LANG and LC_ALL in the CI, and get rid of the LOCALE_OVERRIDE. But it's quite likely that I'm missing something.

In any case, didn't want to work on the localization, this PR is to prepare the tests to work on GitHub Actions, and the raise is an extra to have better error messages if something is wrong in the variable. I'm happy to get rid of it here, but then we may end up with a tuple unpacking error that will require extra debugging to understand what's going on.

@datapythonista
Copy link
Member Author

This won't be needed when/if #29883 is merged, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Error Reporting Incorrect or improved errors from pandas Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants