Just for curiosity's sake, I checked the master branch of numpy, and it looks like they got rid of this stuff, too.

We should probably have someone check the build on Windows.

Numpy still has it -- they just use a union instead of a cast to get the job done:

https://github.com/numpy/numpy/blob/master/numpy/core/include/numpy/npy_math.h#L30

I considered just using Numpy's stuff here, but there is a nameclash with the C++ standard library when you import it, so rather than hacking around that, I decided to just use the C++ standard library.

I can confirm this works and gets rid of a bunch of complier warnings.

Im 👍 however according to http://en.cppreference.com/w/cpp/numeric/math/isfinite isfinite is a c++ 11 feature so depending on how old compilers we want to target it might be a bit to soon?

I bet isfinite has existed in gcc longer....maybe not in the std:: namespace though.

C99 has isfinite in math.h. Just checked on an ancient linux ppc64 system.

On Mon, Jun 15, 2015 at 11:45 AM, Ryan May notifications@github.com wrote:

I bet isfinite has existed in gcc longer....maybe not in the std::
namespace though.

—
Reply to this email directly or view it on GitHub
#4527 (comment)
.

Right -- there's the C99 one, but that seems to get shadowed by the C++ one (at least on gcc 5.1), which is why the numpy ones aren't usable for us. Anyway, at least we're honing in on a solution. Finding the optimally portable one may take some work.

I have access to a RHEL 5 system with gcc 4.1.

Will check there when I get a chance

Given that this is needed now only in C++ code, using the C++ stdlib looks like the cleanest solution. Historically, the need for this functionality arose in C code--still the case in numpy--hence the use of a cast or union. Between these two, the union seems like the right way to do it, and presumably it still works fine in numpy. Even there, it is the legacy of older compilers and standard libraries.

@tacaswell, thanks for tracking this down--not a fun way to spend your weekend. And @mdboom, thanks for finding the right solution.
The only question seems to be whether we need to support compiler/library combinations that are too old to provide stdlib::isfinite. SunOS systems might be in that category.
If we wanted to be extra-conservative, the MPL macros could be retained, but implemented via stdlib when available, and via the numpy code otherwise.
What's a good way of determining whether this level of conservatism is necessary? One way is to merge the present PR and release it; if anyone trips over it, the worst that can happen is that their attempt to upgrade mpl fails and they fall back to the previous release until we put back the macros.

@efiring: The standard way to support multiple compiler generations is to do some configure-time tests -- where you try to compile some code and if that fails, fallback to a different approach. Numpy has a few of these in their setup.py based on support for doing that kind of thing in their custom version distutils. I hate to add that kind of complexity to our build (either by calling into Numpy distutils or copying parts of it), but we may be forced to if this hits enough people to be worth the effort.

An alternative is to copy the union approach in Numpy -- but use it always rather than falling back to system calls if available as Numpy does. That may be slower than the C++ stdlib version -- don't know.

Aren't there also flags that will identify compliers? We could take the approach of changing this and then special casing as we discover things that don't work?

I have access to some debian ancient machines I can test this on.

This patch gets rid of 3 test failures for me on gcc 5.1.0.

If we wanted to be extra-conservative

Nope. 😉

One way is to merge the present PR and release it; if anyone trips over it, the worst that can happen is that their attempt to upgrade mpl fails and they fall back to the previous release until we put back the macros.

I like this approach. It keeps us honest. If this affects somebody, what will the error look like? If the error is put verbatim in this PR, the user will find it, understand the problem, and hopefully comment (hello future readers!). At that point, we can look at a more complex solution if necessary.

Merge merge merge 😉 👍

Thanks @efiring. Did anybody find a compiler which didn't have these definitions? If so, please go ahead and paste the error message into the comments of this PR.

I tested it with GCC 4.1 seems to work