array_t overload resolution support

[jagerman]

This makes array_t respect overload resolution and .noconvert() by failing to load when convert = false if the load source isn't already an array of the correct type.

[wjakob]

This is great and the logical step following #643. Regarding the details of this implementation, I was wondering whether this is perhaps a good moment to get rid of the unequivocally disliked forcecast flag. One way to do this would be to e.g. have array_t always do a force cast if called with convert = true and only accept matching arrays if convert=false.

This is basically what your implementation does now, but I would make it more explicit by:

1. adding a convert parameter to array_t::ensure() (with a default value = true to avoid breaking existing code) and a similar change in raw_array_t

2. removing array::forcecast from the ExtraFlags template argument default value and simply setting it to zero

3. deprecating array_forcecast and setting it to zero to make it inactive

What do you think?

[wjakob]

4. ensure() would need to be updated so that it can signal a failure if convert==false.

5. raw_array_t would need to set detail::npy_api::NPY_ARRAY_FORCECAST_ if convert==true

[wjakob]

(now that I've written the above, I am already starting to doubt if it's a good design..)

[jagerman]

Forcecast doesn't mean quite the same thing, though: it enables potentially data-losing conversions (e.g. float to int). I might well want to be able to accept a list of ints, but not a list of floats. For that, I'd want .noconvert(false) (i.e. no .noconvert() at all), but !forcecast.

However, I'm not sure what the rationale is to having forcecast be the default: it would seem to make much more sense as an opt-in rather than opt-out feature.

[jagerman]

(And I see forcecast-by-default-is-a-bad-idea was also @aldanor's very first point in #338).

[wjakob]

The reason that forcecast is there now is really my fault and perhaps a bit of personal bias. I work in the area of computer graphics, where 32 bit floats are usually a quite reasonable precision. Many graphics-type projects will involve single precision arrays -- however, Python floats are 64 bit by default. This causes various annoyances without the forcecast default since it is basically impossible to call any functions without coercing either pybind11 or numpy to accept or convert array arguments.

[aldanor]

With the new noconvert functionality, I too think it's a perfect time to get rid of forcecast. I would be all in for banishing forcecast altogether, or at most, leaving it as an opt-in.

[jagerman]

Perhaps something like:

template <typename T, int ExtraFlags> using casting_array = array_t<T, ExtraFlags | forcecast>

with array_t's default ExtraFlags changed to 0 instead of forcecast?

[jagerman]

How the hell did f25420a break MSVC's successful compilation from the previous commit?

[jagerman]

Oh, 0defac5 got added to master in between. Updating this PR.

[wjakob]

Ah right, sorry -- I should have said something.

[jagerman]

With the new noconvert functionality, I too think it's a perfect time to get rid of forcecast.

Something tells me that you would think that even without the new noconvert functionality. ;)

[wjakob]

(This is now conflicted). One general question we should discuss is how to proceed with the force conversion flag that is there now.

[jagerman]

In the interest of getting this closed off (there was just a reasonable request to move forward in the gitter room), can we hold off on the forcecast issue for now? (This PR is basically just an addition of two lines of code, plus a whole bunch of overload resolution tests.)

I do think the forcecast stuff is something we should change, but I think the discussion in and eventual resolution to #338 is a better place for it than this PR.

(I also don't see how we can do it without breaking backwards compatibility, which so far, I don't think anything currently on master really breaks--at most, there are some potential changes around the overload resolution, but in order to hit those you would need to have existing, non-functioning overloads).

[wjakob]

Ok, sounds good. Merging is fine with me.