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

Add tests to arviz.labels module #1574

Closed
OriolAbril opened this issue Feb 20, 2021 · 8 comments · Fixed by #2095
Closed

Add tests to arviz.labels module #1574

OriolAbril opened this issue Feb 20, 2021 · 8 comments · Fixed by #2095

Comments

@OriolAbril
Copy link
Member

#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.

@madhucharan
Copy link
Contributor

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

@OriolAbril
Copy link
Member Author

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

@madhucharan
Copy link
Contributor

@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

@OriolAbril
Copy link
Member Author

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 OriolAbril added this to the vNext milestone Mar 5, 2021
@OriolAbril
Copy link
Member Author

@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

@Rishabh261998
Copy link
Contributor

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!

@OriolAbril
Copy link
Member Author

Sounds good, let me know if you need any help

@Rishabh261998
Copy link
Contributor

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants