Remove FLOAT_CMP in favor of pytest's NUMBER?

[nstarman]

Pytest offers NUMBER (see in https://docs.pytest.org/en/7.1.x/how-to/doctest.html#using-doctest-options). I think this does everything FLOAT_CMP does.
For backwards compat, we could make FLOAT_CMP just point to NUMBER...

[pllim]

That would require bumping minversion of pytest to 7.1 ? How does one control rtol and atol?

[Cadair]

NUMBER: when enabled, floating-point numbers only need to match as far as the precision you have written in the expected doctest output. The numbers are compared using pytest.approx() with relative tolerance equal to the precision. For example, the following output would only need to match to 2 decimal places when comparing 3.14 to pytest.approx(math.pi, rel=10**-2)

Seems pretty sane? Perhaps we should test it somewhere?

[pllim]

So, we will lose the ability to use these settings?

https://github.com/astropy/pytest-doctestplus/blob/64d38f2b628a35d3a8dc855ae926bbfe531e36cd/pytest_doctestplus/plugin.py#L105-L113

I am actually not sure who is using it but I want to understand all the implications.

[nstarman]

I think yes we will lose these settings, but gain something nicer, which is that each float comparison can have its own atol, as set by the precision in the doctest. Currently with FLOAT_CMP one could include in the doctest too many significant digits and still match by messing with atol/rtol. With NUMBER, this isn't the case.
The one drawback I could see to switching to NUMBER is if some architecture has a much lower precision than what we test on... but probably a lot of other precision tests in astropy will also be failing so ¯_(ツ)_/¯

[bsipocz]

I would gravitate towards getting on the same page with scipy/numpy rather than vanilla doctest. IMO don't fix what's not broken, and NUMBER doesn't seem to offer any extra over FLOAT_CMP atm.

[pllim]

We discussed this at infrastructure tag-up 2022-08-11. If using NUMBER means one less thing we have to maintain and it is good enough, maybe worth it. Problem is no one has the time to explore this. @nstarman , are you interested to experiment by replacing FLOAT_CMP as alias to NUMBER as a PR here, and then have astropy pick up that branch and see how many tests actually fail?

[bsipocz]

what is the overall time requirement of rewriting all core/coordinated usage of FLOAT_CMP vs keeping maintaining it? E.g. there haven't been that much of issues with it in recent memory.

[pllim]

Re: time requirement -- Not much recently. Though if things ever break, we don't have a lot of resources to deep-dive into what changed upstream, so 🎲 .

[bsipocz]

frankly a feature like # may vary from the numpy/scipy tools would be gamechanger, but I'm not convinced that NUMBER brings anything better to the table.

[pllim]

Haven't heard about # may vary but that sounds promising too. Where is that documented? I cannot find at https://numpy.org/devdocs/reference/testing.html

[bsipocz]

Longish discussion is here and resources (email threads, discorse, repos, etc.) linked from within: numpy/numpy#21070

[nstarman]

are you interested to experiment by replacing FLOAT_CMP as alias to NUMBER as a PR here, and then have astropy pick up that branch and see how many tests actually fail?

😬. I can give it a try. I've never delved under pytest's hood.

[pllim]

I think at the NumFOCUS summit, Scientific Python is interested in maybe moving this repo upstream. Does that sound about right, @bsipocz and @rossbar ?

[bsipocz]

Yes, upstreaming is now very much on the table, yet this suggestion still not clear that would make sense without a clear lists of benefits (that overwhelm the need for the the deprecation and refactoring). So I'm very much so on the opinion of closing this as a "not now".

[Cadair]

upstream as in to pytest?

[bsipocz]

More like a sidestream (still as a plugin, wasn't that the conclusion of the upstreaming discussion that it should still stay as a plugin?) if it works out and is adopted for the more generic libraries.