Add entrypoints to setup.py

[joshmoore]

0.10.0 has broken conda-forge builds. Propose for a quick 0.10.1 release.

see:

• numcodecs v0.10.0 conda-forge/numcodecs-feedstock#86
• Entrypoints #300
• Add entrypoints requirement in next release conda-forge/numcodecs-feedstock#82

TODO:

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

[joshmoore]

Looks like the tests aren't passing on Windows, @martindurant.

  ==================================== ERRORS ====================================
  __________________ ERROR at teardown of test_entrypoint_codec __________________
  
      @pytest.fixture()
      def set_path():
          sys.path.append(here)
          numcodecs.registry.run_entrypoints()
          yield
          sys.path.remove(here)
          numcodecs.registry.run_entrypoints()
  >       numcodecs.registry.codec_registry.pop("test")
  E       KeyError: 'test'
  
  /tmp/tmp.WpzI00cMS0/venv/lib/python3.7/site-packages/numcodecs/tests/test_entrypoints.py:20: KeyError

[martindurant]

I guess we can always pop("test", None), but why isn't it there in the first place? Perhaps this test would do well to interrogate the state of numcodecs.registry.entries.

[joshmoore]

@martindurant, that subsequently fails since the codec isn't registered:

  >       raise ValueError('codec not available: %r' % codec_id)
  E       ValueError: codec not available: 'test'

I assume the fixture isn't setup to be imported on Windows.

[martindurant]

Tried it on ubuntu-under-WSL and I don't see any error. I can't tell, but since the CI job uses bash, I assume I am doing the same thing it is (i.e., not gitbash or something weird).

[joshmoore]

Apparently is was also previously failing on the other platforms, @martindurant. Now it's only failing on ubuntu and macos. 🤷🏽

[martindurant]

I just can't get it to fail :|
With this very branch and a new fresh env and steps as given in the ci script; all works fine. Is there a new version of entrypoints?

[joshmoore]

wheel.yml doesn't look to use any of the requirements files so should be only using setup.py.

[jakirkham]

This might not be needed. Just trying something 🍀

That said, we probably already should have had this given we have shared libraries

Or at least shared libraries misbehaving was what had usually prompted this before. Maybe it is not as relevant anymore

[joshmoore]

Knowing this is breaking condo-forge space, thoughts on getting the addition of entry points to setup.py released and then working on a test solution? Will condo-forge sufficiently test that numcodecs is being installed appropriately?

[martindurant]

You define your own tests in conda-forge. Some packages just test that the imports work, others run the whole test suite.

[jakirkham]

We run the full test suite in conda-forge

Edit: Think to Josh's point, we run pip check, which should test the requirement is included by the package when installed

[martindurant]

Should we, in the end, ignore entrypoints and use builtins only? Like the following:

out = []
for dist in importlib.metadata.distributions():
    eps = [ep for ep in dist.entry_points if ep.group == "numcodecs.codecs"]
    if eps:
        out.extend(eps)

This takes 0.1s on my bloated env, and we only do it once, all taken up by path-string manipulating and config file parsing; however, nothing is cached, so other packages that do this kind of thing (e.g., intake) would end up repeating the work.

[joshmoore]

@martindurant @jakirkham thoughts on xfailing like this?

[martindurant]

Well, OK; but it would be good to know if any real situations face this problem! I suppose we may start to see reports as people use entrypoints-based codecs, perhaps via kerchunk.

[joshmoore]

Definitely! But the current situation is breaking downstream already.

[martindurant]

Sounds like you should hit the big green button, @joshmoore

[jakirkham]

It might be worth configuring automerge and/or disabling the requirement to update PRs to latest. Just a thought 🙂

[joshmoore]

Created #343

Merging.