Add "where" based ufunc masked array support decorator

[DocOtak]

This PR replaces the match_args_return decorator in _wrapped_ufuncs with the one proposed/discussed in #97 (comment) .

I don't think it's ready for merging: needs tests, more discussion, and the non ufunc functions need to be considered. But I wanted to show what a change like this might look like.

[efiring]

This certainly looks more elegant than what I originally came up with. I'm entirely open
to making the switch once it's clear it won't break anything.

[ocefpaf]

@DocOtak and @efiring I believe that we should mint a new release with the latest changes and fixes. Should we wait to include this PR in it or should we release it now and mint a fresh one when this is done?

[efiring]

I think it makes sense to release both GSW-C and GSW-Python now.

[DocOtak]

I agree, don't wait on this PR.

[ocefpaf]

I think it makes sense to release both GSW-C and GSW-Python now.

Done and done!

[DocOtak]

I am interested in continuing work on this, but would appreciate some guidance. This decorator does not work with the non ufunc functions we have.

Reading through my code comments, I think I should at least make it clear that the where=True ufunc default is the default of all ufuncs by numpy definition, not just here in this decorator. i.e. if you don't pass a where arg, numpy passes True for you.

[efiring]

I'm inclined to merge this. Are there any dangers? Is there any real problem with the need to keep the old decorator for the non-ufuncs, at least initially?

[DocOtak]

A danger I can think of is the function signature change with the **kwargs being added, this being "greedy", but it does just do pass through to the wrapped ufunc, which should be safe, but I don't currently have any good ideas about what tests would look like.

This would allow any of the ufunc optional keyword arguments to be passed through, so in some ways, might be a bit nicer/cleaner wrapping of the ufuncs.

I don't see any places where masked array support is tested at all. Maybe there could be a parallel set of ufunc tests that have random (fixed seed?) masks applied and then checking that the output has the expected ORed together mask?

My updates yesterday were just trying to get it rebased so it was in a merge-able state, since I was in the code anyway. Due to changes in the wrapped ufunc generators, was more tricky than I thought it would be.

[efiring]

Regarding explicit masked array testing: I don't think we need to put in random masked points. Most of the testing is done with the test data and results from the Matlab version, which already has some NaNs. It should be sufficient to use "masked_invalid" to generate masked array inputs, which should then yield the same results as the original after converting the results back with "ma.filled()".

[DocOtak]

@efiring How are you feeling about this one? I don't see a problem with keeping the other decorator around for the non ufunc methods that still need it.

[efiring]

I like the simplicity of it, and I can merge it. Looking at it again, however, you could consider some reorganization that I think would make it even simpler and easier to read. If you move the "has_masked_arrays" to the very top of the wrapper, you could then do

if not has_masked_arrays:
    return f(*args, **kwargs)

That makes it immediately apparent that the wrapper has an effect only when there are masked arguments.
Then no more has_masked_arrays checks are needed, the remainder is simplified, losing one level of indentation. (I might be missing something, of course.)

You could also do a minor cosmetic cleanup by adding capitalization and punctuation to the comments.

[DocOtak]

Good idea with the inverting of the has_masked_arrays check to remove a level of indentation.

PR should be updated, I did do a rebase again main so I could run the tests locally with everything that's happened in the last few months.

[ocefpaf]

I'll mint a new patch release with these changes ASAP.

[efiring]

Thank you, @DocOtak and @ocefpaf.