sandialabs / pyGSTi

A python implementation of Gate Set Tomography
http://www.pygsti.info
Apache License 2.0
134 stars 56 forks source link

Missing options in pytest configuration file (pytest.ini) #361

Closed rileyjmurray closed 10 months ago

rileyjmurray commented 11 months ago

Many of our tests are in classes titled <my_class_name>Tester. By default, PyTest expects a naming convention of the form Test<my_class_name>. PyTest can be configured to automatically discover the tests we have by an appropriate change of pytest.ini. Making this change by branching off from develop at this commit seems to result in more errors when running pytest test/unit.

We should definitely make this change to pytest.ini. I'm filing this issue so that we can track the consequences of making this change, including any specific failing tests that need to be addressed.

What I've seen so far

For the curious, here are the errors I get with pytest test/unit immediately after branching from the aforementioned commit:

Pasted Graphic 6

I'll note weirdness where PyTest did find some tests with the Tester suffix. However, here are the errors I get when running pytest test/unit after changing pytest.ini:

image

The screenshot above suggests that making the proposed change to pytest.ini helps PyTest discover another 25 tests that are failing.

rileyjmurray commented 11 months ago

I've done some looking into why PyTest found some classes with the Tester suffix before I changed pytest.ini. I think this has something to do with class inheritance structures.

Here are the full names used in definitions of relevant classes.

So it seems that with the current pytest.ini, PyTest automatically finds classes with the Tester suffix if (and only if) the class inherits from unittest.TestCase. If we change pytest.ini then PyTest finds all classes with the Tester suffix.

(For reproducibility's sake, I'll note that I'm using PyTest 7.4.3.)

rileyjmurray commented 11 months ago

GitHub actions tests are using older version of an optional dependency called "qibo" (v 0.1.7). See here for the qibo version in the passing the test logs. Using qibo 0.2.1 causes tests to fail at this commit without any changes to pytest.ini.

Looks like I would have gotten the older (correct / safe) version of qibo if I installed pyGSTi with pip install -e .['testing']. Unfortunately for me, I installed pyGSTi as follows:

pip install -r requirements.txt
pip install -r optional-requirements.txt
pip install -e .

The difference is that optional_requirements.txt doesn't enforce a version bound on qibo.

If I had installed with pip install -e .['testing'] then I also would have gotten the csaps optional dependency. Luckily it's easy to modify the failing test in InterpygateConstructionTester to gracefully handle when csaps is not installed. Note: csaps is not listed in optional-requirements.txt.

rileyjmurray commented 11 months ago

We might want to add a version check for qibo at runtime. We need to do a similar thing in CVXPY when we have to check if a third-party numerical solver has some feature. I'm not sure where we'd place the check for qibo, though.

Edit: opened #363 to document that desire.

sserita commented 10 months ago

Closed with release of 0.9.12.