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

Fix np.MachAr warning matching in test. #9362

Merged
merged 1 commit into from Dec 14, 2023
Merged

Conversation

sklam
Copy link
Member

@sklam sklam commented Dec 14, 2023

Somehow #9347 changed the behavior of the filter.
The invisible characters from termcolor highlight now matters. But comparing the warning message before and after that PR does not reveal any visible difference.

Also added a simplefilter("ignore") to suppress any warning config from PYTHONWARNINGS in the python process.

Somehow numba#9347 changed the behavior of the filter.
The invisible characters from termcolor highlight now matters.
But comparing the warning message before and after that PR
does not reveal any visible difference.

Also added a simplefilter("ignore") to suppress any warning
config from PYTHONWARNINGS in the python process.
@sklam sklam added the skip_release_notes Skip towncrier requirement label Dec 14, 2023
@sklam
Copy link
Member Author

sklam commented Dec 14, 2023

To test this, make sure the environment has colorama and NUMBA_DISABLE_ERROR_MESSAGE_HIGHLIGHTING is cleared.

Run: NUMBA_DISABLE_ERROR_MESSAGE_HIGHLIGHTING= SUBPROC_TEST=1 python runtests.py -k test_np_MachAr_deprecation_np122 -v

@sklam
Copy link
Member Author

sklam commented Dec 14, 2023

BFID: numba_yaml_423

@sklam sklam added the Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm label Dec 14, 2023
@sklam sklam closed this Dec 14, 2023
@sklam sklam reopened this Dec 14, 2023
@sklam sklam marked this pull request as ready for review December 14, 2023 04:16
@stuartarchibald
Copy link
Contributor

Thanks for fixing this @sklam. I debugged this a bit, AFAICT the actual strings in the message are the same, what I think is changing is what happens internally as part of the matching process, now the Numba warnings inherit from the standard library warnings the way the message strings are normalized internally is different:
https://github.com/python/cpython/blob/e0c6995b4fa9803a886fe1d88d6a9aec166a99e8/Python/_warnings.c#L657-L674

Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the patch. I tested main manually against the reproducer in #9362 (comment) and the test fails, application of this patch then makes it pass. The changes also make sense with respect to some debugging I did #9362 (comment).

@stuartarchibald stuartarchibald added 4 - Waiting on CI Review etc done, waiting for CI to finish and removed 3 - Ready for Review labels Dec 14, 2023
@stuartarchibald stuartarchibald added this to the 0.59.0-rc1 milestone Dec 14, 2023
This was referenced Dec 14, 2023
@sklam sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm 4 - Waiting on CI Review etc done, waiting for CI to finish labels Dec 14, 2023
@sklam
Copy link
Member Author

sklam commented Dec 14, 2023

CI passed and enough buildfarm config has passed (e.g just windows ones are taking a long time and we don't want to wait for it)

@sklam sklam merged commit 4619d11 into numba:main Dec 14, 2023
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge skip_release_notes Skip towncrier requirement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants