Allow base with dimensionless quantity as base for np.power. - #4770

Also, needs a change log, but I'll let @mhvk decide whether to backport or not, which affects where the change log entry goes.

I think this is a bug, so, yes, this should be backported. Set milestone accordingly.

But think we need some test cases (3*(u.cm/u.cm))**3 should still give 27; an answer of 27 (cm3 / cm3) is also OK, but I don't think 27 cm/cm is OK, even if it formally is the same thing. Also, as said in the other thread, ideally (3*(u.m / u.cm))** b would work too, including broadcasting.

The (3*(u.cm/u.cm))**3 does return 27 without units.

I've tested it with the additional change:

        if function is np.power and result_unit != dimensionless_unscaled:

            if units[1] is None:
                p = args[1]
            else:
                p = args[1].to(dimensionless_unscaled).value

            try:
                result_unit = result_unit ** validate_power(p)
            except ValueError as exc:
                # Array power; may be OK for dimensionless input.
                try:
                    converters[0] = units[0]._get_converter(dimensionless_unscaled)
                    result_unit = dimensionless_unscaled
                except:
                    raise exc

and it works, sort of. Just when the exponent is a list it yields strange results:

a = np.array([2, 2, 2, 2]) * u.Unit('cm/m')
a ** [3,3,3,4]   # [8, 8, 8, 16] cm /m

but the specified cases seem to work:

(np.array([1,2,3])*(u.m / u.cm))**np.array([2,3,3]) # [10000, 8000000, 27000000]
(np.array([1,2,3])*(u.m / u.cm))**np.array(3)  # [1, 8, 27] m3 / cm3
(3*(u.cm/u.cm))**3          # 27 * u.dimensionless_unscaled

I have to look into that before I push the change.

@MSeifert04 - looks promising! But should get the units right also in corner cases -- there must be something wrong with what I suggested.

Also, is the change of is not to != still needed? I'm still not 100% sure this is right (in the sense that I don't understand why (3*(u.cm/u.cm))**3 becomes dimensionless if the actual resetting of result_unit is skipped).

I'll try to look in a bit more detail tomorrow.

I've investigated a bit and if the exponent is a list, tuple the unit will not be set to dimensionless. For int, float, np.array, np.ma.array and u.Quantity it works fine.

I couldn't locate where it goes wrong but I think the unit get's lost somewhere in __array_wrap__. I may have more time tomorrow but taking care of u.cm / u.m is a bit more complicated than I originally thought.

@mhvk - The != is actually a shortcut in case the base has not a true dimensionless unit but only a composite-dimensionless-unit like u.cm / u.cm. It wouldn't make a real difference (except maybe for the performance).

@MSeifert04 - OK, I tested myself now, and saw what I should have seen before, that u.cm/u.cm is considered dimensionless in every way except in is. I did get the larger case of m/cm working (as you thought, it was a problem with __array_wrap__ where np.result_type raises an error on list input, which breaks the whole wrapping), but now I see that there was an explicit test case that ensures that something with units like m/cm cannot be raised to multiple powers. Since this is really a judgment call, and I don't feel that strongly that it should work, I think we should leave that part as is.

Now on the solution itself: yours obviously works, but I wondered if it wouldn't be better to ensure that a composite unit that has no elements left and has scale of unit, is simply set to dimensionless_unscaled (rather than define a new one that is dimensionless as well). But this doesn't seem particularly simple, so unless you, I, or someone else comes up with a good alternative solution, I'll merge this tomorrow...

@mhvk If the explicit test case was inside test_power_array_array3 then the test case was introduced by me. Let me know if you want to submit your solution otherwise I'll supply a changelog-message for this PR.

I think the converting a dimensionless-composite-unit to a dimensionless-unit is a bit too much work for me at the moment.

Yes, that was it. I do have something that works correctly, and removes a bug in the process. Let me do a PR to yours so you can have a look.

@mhvk I had a look and it works very well. I have a few minor comments (especially about catching explicit Exceptions) but actually I don't know how to proceed now: Will you submit your PR or should I merge your PR into my branch add the comments and push? Is that even possible?

@MSeifert04 - I think now it is easiest for you to merge my commit into your branch, which makes it visible here too, and then we can have an additional commit.

I think on your computer's end, you'd do:

git fetch origin
git checkout unitlessBase_notScalarPower_fix
git rebase origin/unitlessBase_notScalarPower_fix

and after that you can edit on your machine and push up as usual.

@mhvk Thank you! With your help this was easier then expected.

@MSeifert04 - OK, now reviewed the additional changes you made -- good! So, merging...

Thanks @mhvk and @MSeifert04 for working on that!

Re-milestoned, as discussed in #4395 - I'll need to move the changelog entry, will do that as part of my consistency checks.