search

mila-iqia / fuel

A data pipeline framework for machine learning
MIT License
867 stars 268 forks source link

Auto-open out-of-memory HDF5 files. #349

Closed dwf closed 8 years ago

dwf commented 8 years ago

Fixes #343.

Realizing now that @rizar proposes a different solution. I'll look at implementing that.

dwf commented 8 years ago

I think we should stop letting the perfect be the enemy of the good and merge this. It's technical debt but it's a very manageable kind that is worth amassing IMHO. I will add a comment to the effect that this is a giant hack that needs to be addressed in some follow-up work to do the "right" thing instead of the expedient thing.

rizar commented 8 years ago

sigh... I don't really have time now to solve it properly, and I guess neither do you. LGTM.

dmitriy-serdyuk commented 8 years ago

Is it ready to merge then?

dwf commented 8 years ago

Yep, seems fine to me (I just pushed a better commit message last night, so assuming all the test still pass...).

On Mon, Jul 11, 2016 at 12:59 PM, dmitriy-serdyuk notifications@github.com wrote:

Is it ready to merge then?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mila-udem/fuel/pull/349#issuecomment-231796491, or mute the thread https://github.com/notifications/unsubscribe/AADrLhUPU2qxk17ukQKjnFhrSM7FWtTBks5qUnZXgaJpZM4IMPa_ .