Skip to content

Fix compatibility with pytest-run-parallel and add free-threaded CI jobs#63

Open
astrofrog wants to merge 2 commits into
mainfrom
free-threading
Open

Fix compatibility with pytest-run-parallel and add free-threaded CI jobs#63
astrofrog wants to merge 2 commits into
mainfrom
free-threading

Conversation

@astrofrog
Copy link
Copy Markdown
Member

Without this fix, we can't run tests with pytest-run-parallel if those tests rely on pytest-arraydiff (which is used e.g. in reproject):

hook_name = 'pytest_runtest_call', hook_impl = <HookImpl plugin_name='140326316292304', plugin=<pytest_arraydiff.plugin.ArrayComparison object at 0x7fa0443c38d0>>
e = AttributeError("'NoneType' object has no attribute 'writeto'")

    def _warn_teardown_exception(
        hook_name: str, hook_impl: HookImpl, e: BaseException
    ) -> None:
        msg = "A plugin raised an exception during an old-style hookwrapper teardown.\n"
        msg += f"Plugin: {hook_impl.plugin_name}, Hook: {hook_name}\n"
        msg += f"{type(e).__name__}: {e}\n"
        msg += "For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning"  # noqa: E501
>       warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=6)
E       pluggy.PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
E       Plugin: 140326316292304, Hook: pytest_runtest_call
E       AttributeError: 'NoneType' object has no attribute 'writeto'
E       For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning

../../../python/dev/lib/python3.11/site-packages/pluggy/_callers.py:73: PluggyTeardownRaisedWarning

The fix here ensures we don't wrap the tests multiple times, and make sure we always pass output values through.

(Note: I pushed this to an upstream branch by mistake as this repo used to be under my user - will fix it for future)

… it has no free-threaded wheel and its sdist requires libhdf5-dev which CI does not install
@astrofrog astrofrog force-pushed the free-threading branch 4 times, most recently from eeb1750 to 340e541 Compare May 28, 2026 20:27
@astrofrog astrofrog requested review from bsipocz and pllim May 28, 2026 21:39
Comment on lines +34 to +35
- linux: py313t-test
- linux: py313t-test-parallel
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd suggest going straight to 314t here:

  • 3.13t was always experimental
  • cibuildwheel is currently sunsetting it so it seems unlikely that tables will ever support it

gen_dir = os.path.join(tmpdir, 'reference')

# Generate the reference file first
code = subprocess.call(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this use the builtin pytester fixture instead ? https://docs.pytest.org/en/stable/reference/reference.html#std-fixture-pytester

It does require pytest>=6.2, which is very reasonable now IMO (6.2 came out in 2020)

Comment on lines +196 to +201
tmpdir = tempfile.mkdtemp()
test_file = os.path.join(tmpdir, 'test.py')
with open(test_file, 'w') as f:
f.write(TEST_PARALLEL)

gen_dir = os.path.join(tmpdir, 'reference')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest using the tmp_path builtin fixture here

Suggested change
tmpdir = tempfile.mkdtemp()
test_file = os.path.join(tmpdir, 'test.py')
with open(test_file, 'w') as f:
f.write(TEST_PARALLEL)
gen_dir = os.path.join(tmpdir, 'reference')
test_file = tmp_path / 'test.py'
test_file.write_test(TEST_PARALLEL)
gen_dir = tmp_path / 'reference'

Comment thread setup.cfg
numpy

# tables limitation is until 3.9.3 is out as that supports ARM OSX.
# tables limitation is until 3.9.3 is out as that supports ARM OSX, and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this additional comment is particularly helpful here; it'd be much more fitting in tox.ini

Comment thread tox.ini
Comment on lines +39 to +40
# which CI does not install. Skip the HDF5 extras on free-threaded
# envs.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aligning the phrasing with the way configuration is written

Suggested change
# which CI does not install. Skip the HDF5 extras on free-threaded
# envs.
# which CI does not install. Only run these test on GIL-enabled builds.

Comment on lines +387 to +388
@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_call(self, item):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn't this implementation now exactly equivalent to the default ?

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.

2 participants