Change all xfail decorated tests currently xpassing so that they all fail

[matteo-pallini]

Description of the problem

Some tests are expected to fail if certain conditions are met. Given that this is expected these tests are marked as expected to fail through the use of the decorator @pytest.mark.xfail.

Some of xfail tests are not always running with the same conditions. These may change for every run and time to time generate unexpected passes rather than failures.

The check_logcdf check for the normal distribution is an example affected by this behavior (although in the current version of the codebase it's not xfailed, it will be). The non-deterministic component in this case, is introduced by the sampling of the points to be tested from the distribution values domain (n_samples set to 100).
https://github.com/pymc-devs/pymc3/blob/d248a0e94d67f12c67342c54f90f9b72752e1a1b/pymc3/tests/test_distributions.py#L892-L906

Potential Solution

Change all xfail decorated tests so that they always fail, no matter what conditions are in place. Also set all the @pytest.mark.xfail tests to @pytest.mark.xfail(strict=True) so that any xfail test unexpectedly passing will result in an error in the test run.

A set of tests that should probably be checked are all the ones using any sampling. ie any test running check_logcdf, check_logp, check_selfconsistency_discrete_logcdf, check_int_to_1 (and any other test using these functions). It is possible to test whether these tests ever result in a pass by setting n_samples=-1, which will run the test for all possible cases in the domain

[matteo-pallini]

@ricardoV94 @michaelosthege I created an issue for changing all the @pytest.mark.xfail and set them to strict=True. Also, happy to work on that, unless there is something else higher priority.

[michaelosthege]

@DRabbit17 over in #4512 I'm adding this section to pyproject.toml:

[pytest]
xfail_strict=true

My PR targets the v4 branch. In my opinion it should be sufficient to resolve it for v4?

if you want to resolve it for the master branch please do it with the same config setting. But I also think it's time to let go of v3 and focus on v4.

[matteo-pallini]

@michaelosthege thanks for the heads up.

I thought that V4 development had been stopped.
https://pymc-devs.medium.com/the-future-of-pymc3-or-theano-is-dead-long-live-theano-d8005f8a0e9b

Is it actually not the case and it's better to focus on V4?

[michaelosthege]

@DRabbit17 thanks for asking - we should update that article with a big ⚠️ because plans have changed since.

Please refer to https://github.com/pymc-devs/pymc3/wiki/Timeline which is I will also update again to reflect that we now have a v3 branch for backporting critical bugfixes.

[michaelosthege]

@DRabbit17 correction: with v4 we refer to the development branch for PyMC3 v4.0.0.

[matteo-pallini]

@michaelosthege makes sense, thanks for clarifying and for pointing me to the timeline page, that's handy.

I tried to investigate xpasses on v4. Unfortunately, a large number of tests seem to generate errors at the moment (eg test_ndarray_backend has 148 errors, while on master there is none). I feel that it may be more efficient to investigate and fix xpasses once v4 is more stable. So, I would put this on hold for now. Let me know what you think.

Also, as far as I understood issues in vNext v4 are high-priority at the moment. Would you advise for any of them which is beginner-friendly enough and still free?

[michaelosthege]

@DRabbit17 excellent question. In addition to the vNext v4 milestone you can also filter for v4 labels.

• You can git cherry-pick the changes from my fix-v3-4528 branch in Bring back stats and plotting aliases #4536 into a second PR for the master branch. There we'll want to convert the warnings.warn into raise in order to follow-through with the DeprecationWarning.
• Combine Windows and Pytest workflows using YAML anchors #4517 would be useful to improve the Windows support - it's mostly about dealing with YAML and probably putting in (a lot of ) pytest.mark.xfail. Best worked on if you are on Windows yourself.
• String (and graph) representations in v4 #4494 requires some digging, but is definitely important
• Refactoring distributions for v4. Similar to Refactor basic Distributions #4508

[matteo-pallini]

Thanks for sharing a list of suggestions, that's very handy.

I spent some time getting a bit more context on the first point you mentioned, replacing the warnings.warn with raise in case the old naming for the plotting functions is used. I should be able to share a PR for that in a few days

[michaelosthege]

Thanks for sharing a list of suggestions, that's very handy.

I spent some time getting a bit more context on the first point you mentioned, replacing the warnings.warn with raise in case the old naming for the plotting functions is used. I should be able to share a PR for that in a few days

Sorry that I went ahead with that first item - it was dealt with in #4549 already. I hope you can at least inspect the diff there to learn a thing or two anyway.

[matteo-pallini]

No prob, thanks for sharing the PR

I will give a look at the last bullet point then and if still free work on that