Skip to content

np.ma.arrray(['string', np.ma.masked]) gives FutureWarning after #18116 (breaks astropy) #18425

Closed

Description

@taldcroft

#18116 caused the astropy regression tests to start failing with numpy-dev, see astropy/astropy#11291.

Reproducing code example:

>>> import numpy as np
>>> np.ma.array(['val', np.ma.masked])
/Users/aldcroft/git/astropy/.tox/test-devdeps/lib/python3.7/site-packages/numpy/ma/core.py:2826: FutureWarning: Promotion of numbers and bools to strings is deprecated. In the future, code such as `np.concatenate((['string'], [0]))` will raise an error, while `np.asarray(['string', 0])` will return an array with `dtype=object`.  To avoid the warning while retaining a string result use `dtype='U'` (or 'S').  To get an array of Python objects use `dtype=object`. (Warning added in NumPy 1.21)
  order=order, subok=True, ndmin=ndmin)
masked_array(data=['val', --],
             mask=[False,  True],
       fill_value='N/A',
            dtype='<U32')

The expected result is the same masked array but with no FutureWarning (from numpy 1.18.5 in this case):

>>> import numpy as np
>>> np.ma.array(['val', np.ma.masked])
masked_array(data=['val', --],
             mask=[False,  True],
       fill_value='N/A',
            dtype='<U32')

Things get a little confusing for tests that are run in pytest because when warnings are turned into errors then there appears to be a try/except within numpy which triggers immediate conversion to object:

(test-devdeps) (astropy) ➜  astropy git:(master) python -Werror                                         
Python 3.7.9 (default, Aug 31 2020, 07:22:35) 
[Clang 10.0.0 ] :: Anaconda, Inc. on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy as np
>>> np.ma.array(['val', np.ma.masked])
masked_array(data=['val', --],
             mask=[False,  True],
       fill_value='?',
            dtype=object)

NumPy/Python version information:

>>> import sys, numpy; print(numpy.__version__, sys.version)
1.21.0.dev0+759.g7a18e4ac4 3.7.9 (default, Aug 31 2020, 07:22:35) 
[Clang 10.0.0 ]

Activity

  1. eric-wieser commented on Feb 16, 2021

    @eric-wieser
    Member

    This looks like an unintended warning to me, and is just a case of a deprecation causing an ugly hack in np.ma (namely, that np.ma.masked is a float64 scalar) to surface.

  2. seberg commented on Feb 16, 2021

    @seberg
    Member

    Hmm, we can avoid the FutureWarning being eaten. I was aware of it, but it is more of a problem then I wanted to admit.

    I am not immediately sure what to do about the string + masked combination problem. Sure np.ma.masked should not be a float64, but touching that seems like a rabbit hole.

  3. added this to the 1.21.0 release milestone on Feb 16, 2021
  4. pllim commented on Feb 26, 2021

    @pllim
    Contributor

    Hello. Is there a time line for this fix? Thanks!

  5. seberg commented on Feb 26, 2021

    @seberg
    Member

    No, I have not looked into it. I am somewhat hoping that we can create a (silly) Masked DType for this purpose. But even then, I am not sure whether we should consider np.ma.array([np.ma.masked, 3]) returning integers a bug-fix or not.

    Next problem is that I may not be able to do that until promotion is settled, which it is not. And even if we go that way, I am unsure if there will be some reason why it "casts" the masked to NaN which could be another can of worms.

    On the other hand, for NumPy proper, I think the deprecation is important, so...

  6. seberg commented on Feb 26, 2021

    @seberg
    Member

    @pllim or are you interested mainly in getting the FutureWarning at least raised? That I can do quickly, I had even a start on it.

  7. pllim commented on Feb 26, 2021

    @pllim
    Contributor

    @seberg , I'll have to defer to @mhvk and @taldcroft on this. I am mainly asking because of astropy/astropy#11291 and worry that if Numpy releases with this unfixed, it is going to completely fail astropy's CI downstream.

  8. mhvk commented on Feb 26, 2021

    @mhvk
    Contributor

    I worry quite a few more packages than astropy use np.ma.masked as a placeholder for a masked entry of any type, but I can see that this is not exactly easy to solve -- and am very much in favour of the general principle that one does not auto-convert numbers to strings, etc.:

    In [2]: np.ma.array([np.ma.masked, 0, 'val'])
    Out[2]: 
    masked_array(data=[--, '0', 'val'],
                 mask=[ True, False, False],
           fill_value='N/A',
                dtype='<U32')
    

    Still, maybe for now raise the FutureWarning but ensure the auto-conversion to object never happens?

  9. seberg commented on Feb 26, 2021

    @seberg
    Member

    We could probably go always to error, arguably there is some faint thoughts that it would be a good idea in general to require a flag for "on promotion error just use object". Its a bit interesting, for example np.ma.array([np.datetime("2017"), np.ma.masked]) already returns an object array.

    In any case, I plan to try the "special dtype", so that np.ma.array can call np.core.multiarray._multiarray_umath._discover_array_parameters (or np.asarray maybe) with dtype=IgnoreMasked, the only problem is whether that should also ignore the datetime issue, and change the "length" of the string array (note the U32 even without the 0, which comes from the double precision np.ma.masked).
    But, right now figuring out how to implement "promotion" exactly is driving me insane, and that is a prerequisite for such a try.

  10. taldcroft commented on Feb 26, 2021

    @taldcroft
    Author

    To reiterate, I'm interested in the case that is specifically highlighted in the astropy docs as a nice feature:

    >>> a = [1.0, np.ma.masked]
    >>> b = [np.ma.masked, 'val']
    >>> Table([a, b], names=('a', 'b'))
    <Table length=2>
      a     b
    float64 str3
    ------- ----
        1.0   --
        --  val
    

    Under the hood those are just calling np.ma.array on the list inputs.

    @seberg - is your "special dtype" effort going to fix the regression so this continues to work with no astropy code changes?

    In the worst case we can always catch that FutureWarning internally in astropy and put this into an astropy bug-fix release that goes out before the next numpy release. This will work as long as numpy leaves the FutureWarning in place instead of converting to an error there.

  11. seberg commented on Feb 26, 2021

    @seberg
    Member

    Yes it should fix the regression (and more, potentially much more). But considering on how shaky ground np.ma is build here (out of need, not choice), I would not be surprised if a sandworm comes and swallows the idea for some reason.
    Anyway, hopefully I will have tried this in a few weeks...

  12. seberg commented on Feb 28, 2021

    @seberg
    Member

    OK, I have to admit, that most of the things I said were a wrong... For one thing, that part of promotion is already finished. TL;DR: I have to think about it more, I don't think there is a real clear solution and we should likely just hack something in np.ma, input from anyone interested is appreciated.

    We should think about how we can make some of the array-coercion (and other) code easier shareable, e.g. with pandas. And I think it can be achieved probably. But, it is a too big project to solve this small regression probably. And, while I am happy to hack a masked-array special case into the core array-coercion, the DType thoughts themselves are probably in that category as well.

    About the DType thoughts:

    I looked into the masked-array code and that convinced me that this whole thing is basically taped together with a lot of spit and luck, leaving glaring holes that astropy probably just doesn't care about.

    Now, there are two approaches for special DTypes, one would be IgnoreMaMasked as I mentioned. But it would probably require some additional logic in the core. Further, I am not sure it can play with a user provided dtype="S" for example.

    The next one is a "pandas style" masked dtype: MaskedDType with instances MaskedDType(original_dtype) (at which point np.ma.masked would map to a special MaskedDType(None)). That certainly works, but we would at the very least require knowledge in the core to refuse the use of such a DType (it is a dtype, but it is a bit unclear what NumPy should do with it). That probably works, but might run into limits pretty quickly, for one it probably has to be a DType factory rather than a single DType (which should not be not a big problem long term).

    I used to think of the second way as a method of "marrying" the pandas style system with NumPy DTypes, but right now I am not sure. There is also still the possibility of creating "bitpattern" as opposed to "byte- or bit-mask" style masked array-likes. Which also would require a set of DTypes – and would potentially play easier with NumPy. I am not sure that is a problem as such, although it seems that we at least may need some flags for both in any case: Both make sense, but their combination does not. However, that is more a big-picture eco-system problem :).

  13. 1 remaining item

  14. taldcroft commented on Mar 3, 2021

    @taldcroft
    Author

    See astropy/astropy#11364 for a possible fix in astropy, which I hope will keep our tests passing until such time as numpy changes that FutureWarning into an exception. 🤞 :

  15. seberg commented on Mar 3, 2021

    @seberg
    Member

    Thanks, expect we should just fix it in NumPy. We just have to figure out whether to make it a band-aid "fix most occurrences" style fix in np.ma or a more "proper" fix that digs down into the core...

  16. mattip commented on Mar 10, 2021

    @mattip
    Member

    In a developer meeting we discussed this. The general feeling is that this use case is really not a good use of numpy, and we would prefer not to support it. If that is really difficult for astropy, and there is a large demand for this we could reconsider, but we would prefer that astropy actually not create these types of scalar/masked arrays.

  17. taldcroft commented on Mar 11, 2021

    @taldcroft
    Author

    we would prefer that astropy actually not create these types of scalar/masked arrays.

    @mattip - I'm confused about what that means. At face value that sounds like you don't want astropy to ever create masked arrays of strings, but maybe you really mean that you don't want to support initializing a masked array of strings using ['string', np.ma.masked]? We can live without that particular initialization syntax, though it will cause some pain on our end one way or another.

    I'm assuming this will all continue to work:

    In [1]: np.ma.array(['string', 'hello'], mask=[False, True])                                                                                              
    Out[1]: 
    masked_array(data=['string', --],
                 mask=[False,  True],
           fill_value='N/A',
                dtype='<U6')
    
    In [2]: arr = np.ma.array(['string', 'hello'])                                                                                                            
    
    In [3]: arr[1] = np.ma.masked                                                                                                                             
    
    In [4]: arr                                                                                                                                               
    Out[4]: 
    masked_array(data=['string', --],
                 mask=[False,  True],
           fill_value='N/A',
                dtype='<U6')
    
  18. seberg commented on Mar 11, 2021

    @seberg
    Member

    Yeah, that is the the thing. The problem is only really with np.ma.masked.

    Well the consensus was that we don't really want to add special handling into NumPy core, considering how broken np.ma is. (Special DTypes or similar might be a solution, but right now it would be a very rushed one, and we need better idea around NA+DTypes probably.)

    We could try to solve it in np.ma.array(), but considering how broken masked arrays are here, it seemed that it may be more reasonable to work around in astropy :(. But... That does still depend on how awkward it is for astropy, I do not think we are opposed to modifying np.ma.array() if that is easier. (But there also may be reasons why its much less awkward for you, e.g. you may know that all input is 1-D)

  19. taldcroft commented on Mar 11, 2021

    @taldcroft
    Author

    Yeah, that is the the thing. The problem is only really with np.ma.masked.

    @seberg - again (with apologies), can you be a little more clear for me about what precisely that statement means from the astropy perspective? I am not so much interested in numpy internals or the implementation of possible fixes in numpy. I just want to know what calling syntax and functionality will continue to work without warnings and what will not be supported.

    Specifically are the code block examples I gave above going to continue working without warnings for the foreseeable future?

  20. seberg commented on Mar 11, 2021

    @seberg
    Member

    Sorry: yes. Those example would work, things are only problematic if they give a FutureWarning right now. Aside from the initial creation of the masked array, I do not think there is much change. Things like string_masked_array += np.ma.masked would in theory change, but in practice never worked anyway.

    The one thing I can think of that might break is: np.ma.concatenate([arr, [np.ma.masked]]), which maybe be something to look into at least providing a work-around (similar to what concatenate has now).

    If you know that the result is a string masked array, passing dtype="S" will continue to give the same result, but... its probably hard to know in practice :(.

  21. removed
    triage reviewIssue/PR to be discussed at the next triage meeting
    on Mar 12, 2021
  22. charris commented on May 4, 2021

    @charris
    Member

    The time for the 1.21.x branch is approaching. What is the current status of this?

  23. seberg commented on May 16, 2021

    @seberg
    Member

    NumPy master will now always raise the warning when turned into an error. Hopefully, this behaviour is preferential; Of course it doesn't actually fix the problem itself...

  24. removed this from the 1.21.0 release milestone on May 19, 2021
  25. seberg commented on May 26, 2021

    @seberg
    Member

    I am considering trying gh-19101, which could actually provide a path forward here as well... That would allow to opt-in to fall-back to object. OTOH, that isn't actually the current behaviour probably, so...

  26. mattip commented on Sep 7, 2026

    @mattip
    Member

    I got a little lost in the discussion, but the original issue (future warning, when run with -WError the error causes the fill value to change) has been solved by not raising a FutureWarning. Closing. Please reopen or open a new issue if I misunderstood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions