TEOS-10 / python-gsw

Python implementation of the Thermodynamic Equation Of Seawater - 2010 (TEOS-10)
MIT License
45 stars 22 forks source link

handling of masked array inputs #27

Closed ocesaulo closed 7 years ago

ocesaulo commented 7 years ago

Fala filipe @ocefpaf Noticed that some functions were returning unmasked outputs even though input was masked. Traced it to the np.broadcast_array needing an optional argument so input does not loose mask.

Changed all calls to np.broadcast_arrays to include the subok True option; this allows proper handling of masked array inputs.

Also modified Nsquared function to also output salinity/temperature only stratification.

richardsc commented 7 years ago

@ocesaulo wrote: Also modified Nsquared function to also output salinity/temperature only stratification.

Not that I think this is a bad idea, but it does introduce behaviour different from the "official" TEOS-10 function (e.g. in Matlab, C, etc).

Is there a policy on function arguments, behaviours, and return values for the various TEOS/GSW language variants? My sense is that they should be as consistent as possible across all the variants, at least in order to be considered "officially endorsed" TEOS packages.

durack1 commented 7 years ago

@richardsc I believe a policy of default behaviour replicating the matlab master library, and then augmenting these (at least in python) with addition keyword arguments that extend the functionality would be a good way forward. If such augmentations were found to be useful they could be pulled back up the chain to the master code

efiring commented 7 years ago

Changing the output arguments like this is not a good idea--in addition to be surprising and inconsistent, it breaks existing code--so that part of the PR needs to be removed.

richardsc commented 7 years ago

Yes, @efiring, that was my concern.

@ocesaulo In the future it's a good idea to keep pull requests to "single issue" fixes, so that the merits can be discussed (and potentially merged) one at a time. Otherwise it is hard to keep track of the various changes that have been implemented in a PR.

ocesaulo commented 7 years ago

@efiring @richardsc OK, I see. Sorry about that. It was an earlier hack I carried over without much thinking. How do I remove that part now? Should it be closed and resubmitted?

efiring commented 7 years ago

It can all be nicely patched up, so no need to delete anything. I can show you what to do tomorrow. That's probably easier than trying to write everything down here.

PaulMBarker commented 7 years ago

I have only briefly looked at these emails, so I hope I understand what is being said. If you change the programme name, I have no problem if you want to change one of the GSW programmes. I do this all the time, although I do not share those as they are often part of our science work. In fact we are happy to include programmes that others contribute in GSW, if they are well written and scientifically correct. Cowboy/cowgirl science is not encouraged.

ocefpaf commented 7 years ago

@ocesaulo thanks for the contribution! Hopefully you and @efiring can work the details out .

Cowboy/cowgirl science is not encouraged.

Thanks @PaulMBarker. I believe that was never the intention of anyone here.