search

ilastik / lazyflow

lazy parallel ondemand zero copy numpy array data flows with caching and dirty propagation
Other
77 stars 59 forks source link

Import/Export N5 files #279

Closed paulhfu closed 5 years ago

paulhfu commented 6 years ago

This fixes ilastik/ilastik#1788

paulhfu commented 5 years ago

Hey @k-dominik, this and the corresponding, related PR's in ilastik and volumina are ready to be reviewed.

constantinpape commented 5 years ago

I just released z5py 1.3.0. With this the monkey patches should be obsolete.

paulhfu commented 5 years ago

Hey @emilmelnikov, thanks for the review. Concerning the separation of h5 and n5. The first attempt actually was that we had separate classes for each but then decided to merge them. Of course I can still roll that back.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 417

Changes Missing Coverage Covered Lines Changed/Added Lines %
lazyflow/utility/pathHelpers.py 14 16 87.5%
lazyflow/init.py 6 11 54.55%
lazyflow/operators/ioOperators/opExportSlot.py 24 29 82.76%
lazyflow/operators/ioOperators/opStreamingH5N5Reader.py 34 42 80.95%
lazyflow/operators/ioOperators/ioOperators.py 32 43 74.42%
lazyflow/operators/ioOperators/opStreamingH5N5SequenceReaderS.py 45 56 80.36%
lazyflow/operators/ioOperators/opStreamingH5N5SequenceReaderM.py 43 55 78.18%
lazyflow/operators/ioOperators/opInputDataReader.py 23 45 51.11%
<!-- Total: 225 301 74.75% -->
Files with Coverage Reduction New Missed Lines %
lazyflow/operators/ioOperators/opInputDataReader.py 1 60.87%
lazyflow/operators/cacheMemoryManager.py 1 90.68%
<!-- Total: 2 -->
Totals Coverage Status
Change from base Build 413: 0.07%
Covered Lines: 11890
Relevant Lines: 17567
💛 - Coveralls
emilmelnikov commented 5 years ago

Hey @emilmelnikov, thanks for the review. Concerning the separation of h5 and n5. The first attempt actually was that we had separate classes for each but then decided to merge them. Of course I can still roll that back.

I see, maybe it's better to leave them merged (at least for now), because it looks like OpExportSlot needs to be refactored into separate subclasses and a factory.

emilmelnikov commented 5 years ago

It's usually better to use contextmanagers instead of try/finally if the actions in finally are simple, but in this case yes, I don't think it's super necessary to do it.