Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent check for masked array types #2434

Merged
merged 3 commits into from
Mar 15, 2017

Conversation

DPeterK
Copy link
Member

@DPeterK DPeterK commented Mar 13, 2017

Consistency! And using part of an API for what it was meant for (in this case checking that an object is of a certain type).

To wit, anything that looks like the former will now look like the latter:

  • if isinstance(thing, ma.core.MaskedArray) --> if ma.isMaskedArray(thing)
  • if thing is ma.masked --> if ma.is_masked(thing)

@DPeterK
Copy link
Member Author

DPeterK commented Mar 13, 2017

Ping @pp-mo

@QuLogic QuLogic added this to the dask milestone Mar 13, 2017
@DPeterK DPeterK added the dask label Mar 14, 2017
@DPeterK
Copy link
Member Author

DPeterK commented Mar 14, 2017

Tests pass! Anyone want to take a look?

@@ -2221,7 +2221,7 @@ def new_cell_measure_dims(cm_):
data = copy.deepcopy(data)

# We can turn a masked array into a normal array if it's full.
if isinstance(data, ma.core.MaskedArray):
Copy link
Member

@lbdreyer lbdreyer Mar 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ma.core.MaskedArray!! there are quite a few different ways this has been accessed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh our dear old friend lack of consistency...

@DPeterK
Copy link
Member Author

DPeterK commented Mar 15, 2017

It looks you have missed a few

Dammit PyCharm! Your search has let me down...

@DPeterK
Copy link
Member Author

DPeterK commented Mar 15, 2017

Your search has let me down

But then again, perhaps we should blame my ability to regex.

@lbdreyer lbdreyer added this to In Progress in Iris-dask-backlog Mar 15, 2017
@bjlittle
Copy link
Member

@dkillick Awesome, but I think that you're still missing a couple of core.MaskedArray and ma.MaskedArray instances ...

@bjlittle bjlittle self-assigned this Mar 15, 2017
@bjlittle
Copy link
Member

@dkillick Let's get this merged ...

@bjlittle
Copy link
Member

./fileformats/grib/_load_convert.py
1384:        if isinstance(data, ma.core.MaskedArray):
./fileformats/pp.py
100:    if isinstance(data, np.ma.core.MaskedArray):
./experimental/raster.py
934:    if not isinstance(a, (np.ndarray, ma.core.MaskedArray)):
./util.py

and

173:    if isinstance(data, np.ma.MaskedArray):
219:    if isinstance(data, np.ma.MaskedArray):
./pandas.py

These are the obvious candidates ... fix those and I'll merge.

@DPeterK
Copy link
Member Author

DPeterK commented Mar 15, 2017

@bjlittle I've removed a few more examples...

@lbdreyer lbdreyer merged commit 16d2222 into SciTools:dask Mar 15, 2017
@lbdreyer lbdreyer moved this from In Progress to Done in Iris-dask-backlog Mar 15, 2017
@DPeterK DPeterK deleted the consistent_ma_check branch March 16, 2017 14:20
bjlittle pushed a commit to bjlittle/iris that referenced this pull request May 31, 2017
* Consistent checking for ma

* License headers

* Fix missed instances
@QuLogic QuLogic modified the milestones: dask, v2.0 Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants