Repository navigation
Fix MaskedColumn performance issue for mask=None - #7422
Conversation
|
Hi there @taldcroft 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃. Everything looks good from my point of view! 👍 If there are any issues with this message, please report them here. |
|
Good catch! |
|
Please move change log to make bot happy. p.s. Do we need a benchmark? Obviously we can't benchmark with the actual 1.6 GB of data but is it possible to use a smaller dataset? (Just curious.) |
|
@aaryapatil , dare I say that this naturally put your hack pitch to no. 1 place in pyastro 2018 for being the first one completed? 😉 |
|
@pllim @aaryapatil - In addition you also unlocked an extra hackday achievement, made someone else work on your hack idea ;) |
|
@bsipocz , I shall call it "mind hack" 😂 |
|
Thank you all! |
Fix MaskedColumn performance issue for mask=None
Fix MaskedColumn performance issue for mask=None
Fix MaskedColumn performance issue for mask=None
Fixes #7399
I think this could be backported to 2.0.x. As far as I can tell there is no API change, just a performance fix. It could even be called a bug since it is apparently not expected to use
Noneinstead ofFalsefor themaskinMaskedArray(though it gives the same final result, just very slowly).I think we can keep the astropy API for MaskedColumn where
mask=Noneis the default and impliesMaskedArray(data, mask=False). One point is we do not use a default ofnp.ma.nomask, which has the effect of giving a scalarmask, which generally breaks things later.@mhvk - this should be an easy review!
cc @adrn