Fix default entrypoint handling on older Pythons

[vyasr]

The fallback here for updating a dictionary needs to be a dictionary, not a list.

I haven't added any new tests or anything, but I can if I'm pointed in the right direction. Not sure how this library wants to test these types of edge cases with entry points.

Resolves #478

TODO:

• Unit tests and/or doctests in docstrings
• Tests pass locally
• Docstrings and API docs for any new/modified user-facing classes and functions
• Changes documented in docs/release.rst
• Docs build locally
• GitHub Actions CI passes
• Test coverage to 100% (Codecov passes)

[jakirkham]

Thanks Vyas! 🙏

Made some tweaks to handle the release note. Hope that is ok. Please update as needed

Also maybe we should test this somehow. Currently we test the presence of a single entrypoint. Maybe we should also test the absence of any entrypoints?

numcodecs/numcodecs/tests/test_entrypoints.py

Lines 22 to 24 in a5539b5

	def test_entrypoint_codec(set_path):
	cls = numcodecs.registry.get_codec({"id": "test"})
	assert cls.codec_id == "test"

[codecov]

Codecov Report

Merging #475 (aeef761) into main (c28f5c9) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #475   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           56        57    +1     
  Lines         2242      2261   +19     
=========================================
+ Hits          2242      2261   +19     

Files	Coverage Δ
numcodecs/registry.py	100.00% <100.00%> (ø)
numcodecs/tests/test_entrypoints.py	100.00% <100.00%> (ø)
numcodecs/tests/test_entrypoints_backport.py	100.00% <100.00%> (ø)

[jakirkham]

@martindurant , could you please look over this?

[jakirkham]

Was seeing AVX2 issues on macOS builds, that I thought we just fixed with PR ( #479 ). So toggling PR to ensure latest changes in main are picked up

[joshmoore]

OSX failures are still the sporadic:

#error AVX2 is not supported by the target architecture/platform and/or this compiler.

Trying a rebuild on the action itself.

Otherwise, 👍 for the PR.

[vyasr]

Maybe one more rerun? Looks like attempt 2 passed one of the previously failing jobs but the other failed again.

[jakirkham]

Have added a fix in main to disable AVX2 in macOS GHA jobs and merged that into here. That should resolve that issue

[jakirkham]

Since the only entrypoint added is coming from the test entrypoint that we load ourselves. Wonder if we can avoid using a subprocess by simply testing there are no entrypoints present (and effectively those code paths work) before trying to add our own entrypoint (to test that we can load entrypoints). What do you think Vyas?

[vyasr]

Since the only entrypoint added is coming from the test entrypoint that we load ourselves. Wonder if we can avoid using a subprocess by simply testing there are no entrypoints present (and effectively those code paths work) before trying to add our own entrypoint (to test that we can load entrypoints). What do you think Vyas?

I'm afraid I don't understand how your suggestion avoids the problem. My concern is that because importlib_metadata is patching the standard library, as soon as the test importing it runs all subsequent tests in the same process will see the patched library. If there are any other edge cases where simply importing importlib_metadata causes problems, we would start seeing random unexpected failures in other tests based on test ordering (and of course partitioning if tests are run with pytest-xdist). At present, that mostly affects the other test in test_entrypoints.py, so we might be OK just based on pytest's default ordering of tests, but I don't know that we want to rely on that. Also, there's no guarantee that the monkey-patching wouldn't affect any of numcodecs dependencies in unexpected ways.

If you think those cases aren't worth worrying about, though, then I can revert the multiprocessing change.

[jakirkham]

Ah missed we added tests for the backport package importlib_metadata

Ok would it be possible to move these into their own test file? Maybe test_entrypoints_backport.py or similar? We could then also have some top-level check to make sure importlib_metadata is around before testing (ideally without importing given the associated issues)

[vyasr]

Done. The test is now in its own module and only runs if importlib_metadata is available.

[jakirkham]

Would it make sense to reuse set_path here somehow? Should cleanup sys.path and the numcodecs.registry after?

[vyasr]

I thought about reusing, but I think it'd be more work than it's worth. We can't use a fixture directly because it's not supposed to affect the actual test function, we need it to affect the helper function that's run in the subprocess and that function isn't a test so pytest won't patch in fixtures. We could try and split it into a helper function like I had initially, but it's not really necessary. Since the function is running in a subprocess we don't need to clean things up, it shouldn't affect the modules in the parent process.

[jakirkham]

Thanks Vyas! 🙏