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

Testing NamedArray and Variable #8370

Closed
maxrjones opened this issue Oct 25, 2023 · 2 comments
Closed

Testing NamedArray and Variable #8370

maxrjones opened this issue Oct 25, 2023 · 2 comments
Labels
topic-NamedArray Lightweight version of Variable

Comments

@maxrjones
Copy link
Contributor

maxrjones commented Oct 25, 2023

Starting a new issue to continue the discussion from #8244 about the test structure for NamedArray (xref #8238).

In theory it seems like we'd want to test both Variable and NamedArray, but I also don't like the idea of them potentially diverging once we switch to a new repo. OTOH perhaps there won't be much updating of such core functionality.

Maybe we could avoid divergence by importing from the NamedArray test suite and using the same arithmetic tests but on the Variable class?

i'm of the same opinion as @TomNicholas that we can package the shared tests in namedarray, and have the tests for Variable inherit those tests while also including their own specific tests.

It should work to use pytest_collection_modifyitems to collect tests from namedarray that should be also tested in xarray without modification. It's slightly more complicated to add external tests that need to override fixtures, but pytest-dev/pytest#421 (comment) lays out a nice solution (see example pseudo-code below for NamedArray/Variable example). I recognize that this type of property testing would be well suited for Hypothesis, but also expect that would slow down progress too much to be an immediate solution.

@andersy005 when do you think would be the best time to test out this method for migrating tests? I could try it out anytime and don't see many downsides to testing while namedarray is still coupled with xarray.

# namedarray/testing.py

import numpy as np

class NamedArrayTestSuite:
    @pytest.fixture
    def target(self):
        """Fixture that needs to be re-declared"""
        assert 0
    
    @pytest.fixture
    def data(self):
        return 0.5 * np.arange(10).reshape(2, 5)

    def test_properties(self, target, data):
        assert target.dims == ("x", "y")
        assert np.array_equal(target.data, data)
        assert target.attrs == {"key": "value"}
        assert target.ndim == 2
        assert target.sizes == {"x": 2, "y": 5}
        assert target.size == 10
        assert target.nbytes == 80
        assert len(target) == 2
# namedarray/tests/test_namedarray.py

from namedarray.testing import NamedArrayTestSuite
from namedarray import NamedArray

class TestNamedArray(NamedArrayTestSuite):

    @pytest.fixture
    def target(self, data):
        return NamedArray(["x", "y"], data, {"key": "value"})
# xarray/tests/test_variable.py

from namedarray.testing import NamedArrayTestSuite
from xarray import Variable

class TestVariable(NamedArrayTestSuite):
    
    @pytest.fixture
    def target(self, data):
        data = 0.5 * np.arange(10).reshape(2, 5)
        return Variable(["x", "y"], data, {"key": "value"})
@welcome
Copy link

welcome bot commented Oct 25, 2023

Thanks for opening your first issue here at xarray! Be sure to follow the issue template!
If you have an idea for a solution, we would really welcome a Pull Request with proposed changes.
See the Contributing Guide for more.
It may take us a while to respond here, but we really value your contribution. Contributors like you help make xarray better.
Thank you!

@github-actions github-actions bot added the needs triage Issue that has not been reviewed by xarray team member label Oct 25, 2023
@andersy005 andersy005 added topic-NamedArray Lightweight version of Variable and removed needs triage Issue that has not been reviewed by xarray team member labels Oct 26, 2023
@maxrjones
Copy link
Contributor Author

Closed by #8405

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-NamedArray Lightweight version of Variable
Projects
Status: Done
Development

No branches or pull requests

2 participants