toolbox: handle object arrays in pmm (closes #183)

[drsteve]

Closes #183

This PR provides:

• a test for the failures raised in issue Toolbox.pmm does not work with date or object arrays #183
• a trap/workaround for the failure mode (objects can't be determined as finite or not)
• a warning on hitting the trap so the user knows that non-finite values may be present

Any suggestions for wording on the warning? Or other implementation issues?

[TravisBuddy]

Hey @drsteve,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 20dccdd0-fff6-11e9-bdc5-7d173ed77079

[jtniehof]

Rather than arange I would probably use ind = Ellipsis to get the whole array. It makes it a little more obvious what you're doing and dodges any potential problem with multidimensional arrays (I didn't find any in testing, but you never know.)

For the warning, there's little idea of the implications. Unable to exclude infinite values, which may result in wrong output?

This only works for a. If the b args are object type, they still fail. Any reason we can't just rewrite the whole thing as *args and loop for a in args appending to ans each time?

Finally, this doesn't work for strings. That would be a different PR except it does work for objects. Patch attached.

string.txt

[drsteve]

Thanks, updates made.

Your string-type test gave me numpy FutureWarnings like:

 FutureWarning: Conversion of the second argument of issubdtype from `'U'` to `str` is deprecated.
In future, it will be treated as `np.str_ == np.dtype('U').type`.
  if np.issubdtype(a_tmp.dtype, 'S') or np.issubdtype(a_tmp.dtype, 'U'):

So I've also switched that test to

a_tmp.dtype.type in [np.dtype('S').type, np.dtype('U').type]

which should do the same thing, but avoids the warning.

[TravisBuddy]

Hey @drsteve,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 2b9d3c20-002b-11ea-8b9e-5d36b4f25336

[TravisBuddy]

Hey @drsteve,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 8c75c1b0-002c-11ea-8b9e-5d36b4f25336

[jtniehof]

Good catch on the deprecation. Thoughts on the b arguments? I can probably just push to your branch....

[drsteve]

Right, forgot about that...
Why not just get rid of the code for the bs and loop the current code for the a argument over all inputs, appending to the answer list?

If that sounds sensible then I can give it a crack later? If you have other ideas, or wanted to push something then I'm good with that too.

[jtniehof]

That's exactly what I was thinking, just make the function signature *args, start with ans=[], and then for a in args BLAH append to ans, remove the whole "for val in b" loop.

[jtniehof]

Looks good. Since @balarsen hasn't chimed in I'll assume he doesn't object and will merge once Travis is happy.

[TravisBuddy]

Hey @drsteve,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: c6530870-0185-11ea-82f1-cf3ba55cf6d0

[balarsen]

I’m happy.

…

On Thu, Nov 7, 2019 at 1:11 PM Jon Niehof ***@***.***> wrote: Merged #239 <#239> into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#239?email_source=notifications&email_token=AAD4D3MV474YEMOUH63FMPLQSRK5HA5CNFSM4JJF5WT2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOUW4IPQY#event-2780334019>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAD4D3NUYXBHUNRZUCDKZ2DQSRK5HANCNFSM4JJF5WTQ> .