arviz icon indicating copy to clipboard operation
arviz copied to clipboard

Add tests to arviz.labels module

Open OriolAbril opened this issue 4 years ago • 8 comments

#1201 has done a significant refactor of plotting functions to rewrite the labeling process. Part of the code added is tested in the sense that plots use it and tests for plots have passed, but there should be a test_labels.py in the base tests folders to test all bundled classes and test specifically the labellers.

OriolAbril avatar Feb 20 '21 06:02 OriolAbril

Hi @OriolAbril, I went through the discussion and labels.py, So, According to my understanding, the test_labels.py testing function should test the labeller functions/classes to check whether the labels are displayed according to the requirements or not? something like asserting them and checking ? If I am right, I would like to work on it, or please explain the requirement of this issue so that I will continue on the same

madhucharan avatar Feb 26 '21 18:02 madhucharan

Yes, that sound about right. How familiar are you with pytest? Have you checked some of the other test files?

I was planning on adding the skeleton of the test file or at least sharing a template here to suggest how to structure the tests and which fixtures to use, not sure what level of detail would be best

OriolAbril avatar Feb 26 '21 20:02 OriolAbril

@OriolAbril , I have spent some time on the label class , I have some familiarity in writing tests by assertion, checking some particular words,or checking for some files(may be useful for doc string check not here). If there is any template to follow as you mentioned, I will follow the guideline and learn from other tests and implement them.Kindly share the template or some structure of tests

madhucharan avatar Feb 28 '21 13:02 madhucharan

test should be added on a new test file inside the base_tests folder, and I would recommend using a couple module level fixtures that can be used by test functions to ease test writing. There should be some test functions on mix_labellers and then a class (mostly for structuring the code but also to define class level fixtures initializing the labellers) with test functions about the labellers themselves.

I would create test functions that test specific methods of the labeller class and parametrize the test to test all labellers, not functions that test multiple methods of a single labeller. cc @canyon289

from pytest_cases import fixture_ref

@pytest.fixture(scope="module")
def multidim_sels(self):
    # pylint: disable=attribute-defined-outside-init
    class Data:
            sel= {
                "instrument": "a",
                "experiment": 3,
            }
            isel= {
                "instrument": 0,
                "experiment": 4,
            }

        return Data

def test_mix_labellers(multidim_sels):
... 

# possible extra mix_labellers tests

class TestLabellers:
    @pytest.fixture(scope="class")
    def labellers(self):
        return {"BaseLabeller": BaseLabeller(), "IdxLabeller": IdxLabeller(), ...}
        # the MapLabeller should be initialized with some mappings on all levels
        # if we decide to add NoRepeatLabeller as part of ArviZ it should not be tested here but have dedicated test functions due to its special functionality, for now simply skip it

...

    @pytest.mark.parametrize("args", ["BaseLabeller", "theta\na, 3"), ("IDxLabeller", "theta\n0, 4"), ...])
    def test_make_label_vert(self, args, labellers, multidim_sels):
        labeller = labellers[args[0]]
        label = labeller.make_label_vert("theta", multidim_sels.sel, multidim_sels.isel)
        assert label == args[1]

    @pytest.mark.parametrize(...)
    def test_make_label_flat...
        ...

    @...
    def test_make_pp_label...
        ...


    @...
    def test_make_model_label...
        ...

# then maybe also test behaviour when both sel and ìsel are empty dicts, and/or var_name is an empty string

All the other more specific methods will be tested as they are called by the make_label methods.

An alternative would be to remove the class level fixture and then use the following structure when parametrizing:

@pytest.mark.parametrize("args", [(BaseLabeller(), "..."), (IdxLabeller(), "..."), ()])

the con of this approach is that it instatiates the labellers every time the test function is called, instead of only once per class.

OriolAbril avatar Feb 28 '21 15:02 OriolAbril

@madhucharan any news on this? Do you know roughly when will you have time to work on this?

Right now it's not an immediate concern but this should be done before our next release. I'd have no problem working on this now or in a while, but the earlier I know if I'll have to do it the better I can plan around my other commitments

OriolAbril avatar Mar 22 '21 22:03 OriolAbril

HI @OriolAbril I would like to work on this, as I am new to pytest this would help me get familiar with it. Can I be assigned to it, but it would take some time as I am completely new to it. Thanks!

Rishabh261998 avatar Apr 01 '21 17:04 Rishabh261998

Sounds good, let me know if you need any help

OriolAbril avatar Apr 01 '21 18:04 OriolAbril

@OriolAbril, if this issue is an immediate concern, then you could start working on this, as I might not be free for the next two weeks or so as my exams are approaching and also there are some other formalities at home which I need to attend. Sorry for not informing you earlier. Thanks

Rishabh261998 avatar Apr 30 '21 18:04 Rishabh261998