zarr-developers / zarr-python

An implementation of chunked, compressed, N-dimensional arrays for Python.
https://zarr.readthedocs.io
MIT License
1.45k stars 273 forks source link

pytest is an unnecessary dependency? #2068

Closed TomAugspurger closed 1 month ago

TomAugspurger commented 1 month ago

Zarr version

main

Numcodecs version

na

Python Version

na

Operating System

na

Installation

na

Description

https://github.com/zarr-developers/zarr-python/blob/064b2e09b188b8e09536fb327d2c0f5fe8492f2b/pyproject.toml#L35 has pytest as a required dependency (note that this isn't as an dev-only extras dependency).

Looks like that was added in https://github.com/zarr-developers/zarr-python/pull/1785. It wasn't discussed, so it was presumably intended to be a dev-only dependency.

Slightly related, but would you be open to defining tool.hath.envs.test as a standard optional dependency? https://github.com/zarr-developers/zarr-python/blob/064b2e09b188b8e09536fb327d2c0f5fe8492f2b/pyproject.toml#L106 https://hatch.pypa.io/latest/config/environment/overview/#features recommends doing something like

[tool.hatch.envs.test]
features = [
  "test",
]

which should work for both.

Steps to reproduce

na

Additional output

No response

d-v-b commented 1 month ago

we need pytest because of this class

TomAugspurger commented 1 month ago

Thanks. I see that the __init__.py guards against pytest missing: https://github.com/zarr-developers/zarr-python/blob/064b2e09b188b8e09536fb327d2c0f5fe8492f2b/src/zarr/testing/__init__.py#L4-L7. So maybe the intended behavior is to just not have that class be importable if pytest isn't installed?

Here's the behavior I see in an environment without pytest:

In [1]: import zarr

In [2]: import zarr.testing
<ipython-input-2-02d20f6cfc6f>:1: UserWarning: pytest not installed, skipping test suite
  import zarr.testing

In [3]: zarr.testing.StoreTests
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[3], line 1
----> 1 zarr.testing.StoreTests

AttributeError: module 'zarr.testing' has no attribute 'StoreTests'
d-v-b commented 1 month ago

. So maybe the intended behavior is to just not have that class be importable if pytest isn't installed?

This seems right! (@jhamman feel free to issue a correction here).

jhamman commented 1 month ago

Pytest should not be a required dependency so +1 to removing it.