MRCZ io_plugin #1716

robbmcleod · 2017-08-19T21:48:31Z

Couple points to look at:

both the mrc and mrcz plugins support the '.mrc' file extension. I'm guessing you pick the plugin by the
extension somewhere?
The signal axes are correctly ordered but the mixed C/Fortran ordering is a little confusing.
Why no __getitem__ and __setitem__ methods for the metadata attribute?

robbmcleod · 2017-08-20T02:41:36Z

That's strange that TravisCI can't find mrcz for MacOSX. It's a pure-python repo.

ericpre

Just a few comments to get the tests working. I can come back to the rest later.

ericpre · 2017-08-20T14:37:07Z

.travis.yml

@@ -33,7 +33,7 @@ before_install:
 install:

  - if [[ $MINIMAL_ENV == 'False' ]] ; then
-      DEPS="pip numpy scipy matplotlib>=2.0.2 ipython h5py sympy scikit-learn dill natsort setuptools scikit-image cython lxml ipyparallel dask traits traitsui";
+      DEPS="pip numpy scipy matplotlib>=2.0.2 ipython h5py sympy scikit-learn dill natsort setuptools scikit-image cython lxml ipyparallel dask traits traitsui blosc mrcz";


These are the dependencies installed from the conda-forge channel. I cann't find any repo for the mrcz, so I guess it would be better to remove mrcz from here and leave pip install it automatically from pypi when hyperspy is installed.

ericpre · 2017-08-20T14:40:32Z

appveyor.yml

@@ -139,7 +139,7 @@ before_deploy:
  - "ECHO %SCIKIT_IMAGE%"
  # - "%CMD_IN_ENV% wget http://www.lfd.uci.edu/~gohlke/pythonlibs/tuoh5y4k/%SCIKIT_IMAGE% --header User-Agent:Chrome/23.0.1271.97"
  - "%CMD_IN_ENV% pip install %SCIKIT_IMAGE%"
-  - "%CMD_IN_ENV% pip install --upgrade tqdm notebook cython ipython configobj start_jupyter_cm ipywidgets ipyparallel sympy pytest"
+  - "%CMD_IN_ENV% pip install --upgrade tqdm notebook cython ipython configobj start_jupyter_cm ipywidgets ipyparallel sympy pytest blosc mrcz"


Same as for .travis.yml, install blosc from the conda-forge channel with conda and install mrcz with pip from pypi.

ericpre · 2017-08-20T14:52:11Z

hyperspy/tests/io/test_mrcz.py

+mrcName = os.path.join( tmpDir, "testMage.mrcz" )
+MAX_ASYNC_TIME = 2.0
+
+class PythonMrczTests:


This test class is not discovered by pytest, the name needs to start with "Test".

robbmcleod · 2017-08-21T00:48:39Z

Ok, getting closer, JSON version problem now.

robbmcleod · 2017-09-20T21:16:04Z

Looks good to me.

ericpre · 2017-09-22T18:38:23Z

Indeed, it is working fine!
There are two more things worth looking at:

for lazy signal, hyperspy use dask. As it is currently, it is working, except for saving signal (the data are stored as a dask.Array instance). Do you think it would possible to support writing mrcz from dask.Array in python-mrcz?
this PR introduce a non pure python dependency (blosc), which can possibly brake hyperspy installation on some system. For example, I could not install blosc out of the box (install issue? Blosc/python-blosc#127). I am not sure if this issue is solved.

I have tested the Gohlke's blosc binary in the Winpython HyperSpy bundle and it works.

robbmcleod · 2017-09-26T22:06:52Z

I'll look into writing from a dask.Array, it's probably just a matter of calling numpy.asarray().

For blosc, perhaps we can ask Matthew Brett for help. I'll ask Francesc if he would like to move blosc into having manywheels support.

robbmcleod · 2017-09-28T03:08:05Z

Also, quick point: MRCZ doesn't list blosc as a requirement because the import is inside a try block. If blosc isn't present, compression is disabled, but it won't break anyone's install.

ericpre · 2017-09-28T10:42:47Z

Ok great, then we will able to merge this PR even if the blosc wheels/binaries are not ready yet.
Regarding writing from dask.Array, I am not sure if this is relevant here but it may be worth at having a quick look at #1743.

robbmcleod · 2017-09-29T04:45:04Z

I added support for saving from dask.array.core.Array in python-mrcz 0.2.5. My tests suggest the ideal way would be to pull off the chunks one-by-one and compress them but blosc doesn't really support that. c-blosc2 might but it's not Python-aware.

import dask.array as da
import numpy as np
from time import perf_counter
import mrcz

t0 = perf_counter()
foo = da.ones( (50,2048,2048), chunks=(1,2048,2048) )
t1 = perf_counter()
np.array(foo)
t2 = perf_counter()
foo.__array__()
t3 = perf_counter()
for I in np.arange(foo.shape[0]):
    foo[I,:,:].__array__()
t4 = perf_counter()
mrcz.asyncWriteMRC( foo, 'foo.mrcz', compressor='lz4' )
t5 = perf_counter()

print( f"Create da time: {t1-t0:.2e} s" )
print( f"Convert to array: {t2-t1:.3f} s" )
print( f"Call __array__: {t3-t2:.3f} s" )
print( f"Sliced: {t4-t3:.3f} s" )
print( f"MRCZ save: {t5-t4:.2e} s" )

>>> Create da time: 3.96e-04 s
>>> Convert to array: 1.458 s
>>> Call __array__: 0.888 s
>>> Sliced: 0.497 s
>>> MRCZ save: 1.16e-03 s

I'm not sure it's a good idea to use asyncWriteMRC because Dask is pure-python, but it might help if the user calls some GIL-releasing function afterward.

…g it to extras_require.

ericpre · 2017-09-29T18:28:31Z

This is great, thanks @robbmcleod for this nice contribution! I just added the tests for the lazy case and move blosc to extras-require to make it optional. Once appveyor and travis tests are passed, we can merge.

ericpre · 2017-10-09T10:07:25Z

Looking at this travis log, mrcz still depends on blosc.

robbmcleod · 2017-10-10T21:48:58Z

Quick status update: we're in the process of getting multiwheels support for blosc. Hopefully that will be ready by the end of the week.

… (set mrcz>0.3.4 in setup.py).

ericpre · 2018-02-19T20:32:51Z

@robbmcleod, since the master branch of mrcz now raises an ImportError when blosc is not installed, I have fixed the tests accordingly in this PR. If you release a new version of mrcz, then we can merge this PR. Do you have an idea when you will make a new release?

robbmcleod · 2018-02-19T22:21:12Z

The master branch of mrcz 0.3.4 does not raise an ImportError if blosc is missing, it swallows it and disables the compression tools. The mrcz.test() unit tests do generate an error, so is that what you mean?

I just tested it with a fresh conda install, and then installed numpy and then mrcz via pip and it seems to work as intended.

ericpre · 2018-02-19T22:42:05Z

I am talking about this version, which raises an ImportError (only when compression is needed). This is good because it informs the user to install blocs when it is required. If you test this branch with em-MRCZ/python-mrcz@ce67e49, then it all works nicely: the tests catch the error when it is needed and blosc can be an optional dependency.

robbmcleod · 2018-02-20T00:22:11Z

Oh, right, I forgot about that. Let me clean up the docstrings so they meet NumPy standards and I'll release 0.3.5.

robbmcleod · 2018-02-20T05:37:37Z

Eric,

The 0.3.5 release is live now:

em-MRCZ/python-mrcz@3cd4ffe

https://pypi.python.org/pypi/mrcz

robbmcleod · 2018-02-20T18:47:59Z

I see you had a random test failure on the asynchronous tests. You might want to consider re-triggering those builds. I haven't had trouble with them, but weird things can happen between when Python thinks the file write is finished, and when the OS thinks the file write is finished. If you encounter this problem more than once, the only fix I could propose would be a small sleep in-between getting the result from the Future and reading. That or disabling that test.

ericpre · 2018-03-03T11:33:33Z

Yes, it did happen a few times, so I added (#1852) a sleep, as you recommended and so far it seems to do the job.

robbmcleod added 4 commits August 18, 2017 15:42

Initial MRCZ support sans tests

8d785eb

Tested MRCZ plugin

7034aa7

Forgot fixed to the plugin

ca8e02d

Add blosc and mrcz as CI deps

95afc9a

robbmcleod mentioned this pull request Aug 19, 2017

Implement io plugin for MRCZ format #1703

Closed

Update .travis.yml

41e6da7

francisco-dlp added release: next minor status: needs review type: new feature labels Aug 20, 2017

ericpre requested changes Aug 20, 2017

View reviewed changes

robbmcleod added 2 commits August 20, 2017 15:19

MRCZ fix attempt for TravisCI

780f4d6

Remove setup from test_mrcz.py

0788e25

Upgrade MRCZ version req to 0.2.3

a5d4b02

ericpre mentioned this pull request Aug 24, 2017

install issue? Blosc/python-blosc#127

Closed

Autopep8, simplify tests with pytest parametrize feature.

b831514

ericpre added status: WIP and removed status: needs review labels Aug 28, 2017

ericpre and others added 5 commits September 19, 2017 21:18

Set default units to 'nm' in mrcz plugin and fix check Signal type.

ca9f24e

Improve tests.

87dbffe

Mapping gain metadata in mrcz reader.

2d1bf15

Tweak parsing metadata mrcz plugin.

115d154

Unique filenames per datatype

aaf94f1

francisco-dlp added status: needs review and removed status: WIP labels Sep 21, 2017

ericpre added 2 commits September 29, 2017 18:32

Add tests for save load cycle of lazy signals.

e286729

Update setup.py: update mrcz version and make blosc optional by movin…

bc2b87c

…g it to extras_require.

ericpre approved these changes Sep 29, 2017

View reviewed changes

ericpre added status: waiting for author and removed status: needs review labels Oct 9, 2017

robbmcleod mentioned this pull request Jan 19, 2018

Discussion: Storing Big Data #1804

Open

ericpre added 2 commits February 19, 2018 16:42

Merge branch 'RELEASE_next_minor' into RELEASE_next_minor

04f69b0

Fix mrcz test if blosc is not installed, requires mrcz current master…

ce6e78b

… (set mrcz>0.3.4 in setup.py).

Install blosc from pip in appveyor (not only in winpython build).

9d805c3

ericpre merged commit 9142576 into hyperspy:RELEASE_next_minor Feb 20, 2018

ericpre removed the status: waiting for author label Feb 20, 2018

francisco-dlp added this to the v1.4 milestone Apr 13, 2018

ericpre removed release: next minor labels Oct 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MRCZ io_plugin #1716

MRCZ io_plugin #1716

robbmcleod commented Aug 19, 2017

robbmcleod commented Aug 20, 2017

ericpre left a comment

ericpre Aug 20, 2017

ericpre Aug 20, 2017

ericpre Aug 20, 2017

robbmcleod commented Aug 21, 2017

robbmcleod commented Sep 20, 2017

ericpre commented Sep 22, 2017

robbmcleod commented Sep 26, 2017

robbmcleod commented Sep 28, 2017

ericpre commented Sep 28, 2017

robbmcleod commented Sep 29, 2017

ericpre commented Sep 29, 2017

ericpre commented Oct 9, 2017

robbmcleod commented Oct 10, 2017

ericpre commented Feb 19, 2018

robbmcleod commented Feb 19, 2018

ericpre commented Feb 19, 2018

robbmcleod commented Feb 20, 2018

robbmcleod commented Feb 20, 2018

robbmcleod commented Feb 20, 2018 •

edited

Loading

ericpre commented Mar 3, 2018

MRCZ io_plugin #1716

MRCZ io_plugin #1716

Conversation

robbmcleod commented Aug 19, 2017

robbmcleod commented Aug 20, 2017

ericpre left a comment

Choose a reason for hiding this comment

ericpre Aug 20, 2017

Choose a reason for hiding this comment

ericpre Aug 20, 2017

Choose a reason for hiding this comment

ericpre Aug 20, 2017

Choose a reason for hiding this comment

robbmcleod commented Aug 21, 2017

robbmcleod commented Sep 20, 2017

ericpre commented Sep 22, 2017

robbmcleod commented Sep 26, 2017

robbmcleod commented Sep 28, 2017

ericpre commented Sep 28, 2017

robbmcleod commented Sep 29, 2017

ericpre commented Sep 29, 2017

ericpre commented Oct 9, 2017

robbmcleod commented Oct 10, 2017

ericpre commented Feb 19, 2018

robbmcleod commented Feb 19, 2018

ericpre commented Feb 19, 2018

robbmcleod commented Feb 20, 2018

robbmcleod commented Feb 20, 2018

robbmcleod commented Feb 20, 2018 • edited Loading

ericpre commented Mar 3, 2018

robbmcleod commented Feb 20, 2018 •

edited

Loading