Skip to content

biggus#900

Merged
bjlittle merged 5 commits intoSciTools:masterfrom
rhattersley:biggus
Jan 28, 2014
Merged

biggus#900
bjlittle merged 5 commits intoSciTools:masterfrom
rhattersley:biggus

Conversation

@rhattersley
Copy link
Member

This PR switches the handling of deferred data to biggus and gets rid of the data manager concept.

NB. To minimise the change, LazyArray is still used within AuxFactory and for deferred coordinate values.

@shoyer
Copy link
Contributor

shoyer commented Dec 22, 2013

My favorite part about this PR is that it removes twice as many lines as it adds! 👍

Of course, biggus is a major dependency, but I think this removes significant complexity from Iris -- which is a major plus for people like me, who are excited about figuring out how to add new features.

lib/iris/cube.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really want to switch from np.asarray to np.asanyarray? The advantage of asarray is that it only lets through direct instances of ndarray, which guarantees that there aren't any funny ndarray subclasses in use (like matrix), which might overload operators in unexpected ways. In my opinion, asarray is a safer way to go, because it means that people writing math operations on cubes know exactly what they're getting.

Copy link
Member Author

Choose a reason for hiding this comment

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

I used np.asanyarray because I wanted to let masked arrays through. But that could be handled with its own explicit clause.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, okay. We usually use standard ndarrays using np.nan for masked values (because it's faster and more memory efficient), but I know Iris uses numpy's masked array module in many cases.

@pelson
Copy link
Member

pelson commented Dec 24, 2013

I've added several comments on top of b0034a6. Some are memory joggers or questions, others are review actions. Given the number of changes, please only append commits at this point.

In terms of getting this in, I wonder if we need more documentation of biggus (or at least an uptodate pip install). What are you're thoughts on that front.

Hope you have a good Christmas break.

lib/iris/cube.py Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I know you haven't changed this, but you've changed everything else so here goes. The beginning of this sentence now appears somewhat contradictory to the first paragraph. When would the object not contain phenomenon values? I'd also add that it can be a biggus array in here too.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've tried to update this whilst keeping the benefits from #862. I hope @ajdawson and @jkettleb are happy with the result!

@esc24
Copy link
Member

esc24 commented Jan 2, 2014

I'm all done. This is very impressive work 👏 which should make the manipulation and aggregation of large data sets become so much simpler. @pelson raises a number of points that need addressing and I think I've spotted a few small ones too (in particular the fill_value and the logic in merge()), but this is close to being merged in my opinion.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants