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

fix mrc import axes #52

Merged
merged 3 commits into from
Apr 29, 2022
Merged

Conversation

martinschorb
Copy link
Contributor

tests are fine wrt axis mess because we use the same library for writing and reading the files.

@martinschorb
Copy link
Contributor Author

For me, locally the tests went through. So I didn't check that particular one. I'll have a look. I suspect the indeces with the swapped axes might cause it to fail.

@constantinpape
Copy link
Owner

For me, locally the tests went through. So I didn't check that particular one. I'll have a look. I suspect the indeces with the swapped axes might cause it to fail.

Do you have mrcfile installed in the environment where you run the tests? Without it the tests are just being skipped (because I don't want to add it as a hard dependency since for me this is a legacy file format...)

@martinschorb
Copy link
Contributor Author

Do you have mrcfile installed in the environment where you run the tests? Without it the tests are just being skipped (because I don't want to add it as a hard dependency since for me this is a legacy file format...)

Yes, I have it. But it could be that I had some array left in memory when checking or alternatively forgot to manually call that particular test function. I am not very familiar with the unittest classes as I usually use pytest, but I'll give it a try.

@constantinpape
Copy link
Owner

Yes, I have it. But it could be that I had some array left in memory when checking or alternatively forgot to manually call that particular test function. I am not very familiar with the unittest classes as I usually use pytest, but I'll give it a try.

Just do python test_mrc_wrapper.py and it will run all tests in the file.

@martinschorb
Copy link
Contributor Author

Just do python test_mrc_wrapper.py and it will run all tests in the file.

Yes, that's what I did and it went through. Otherwise I wouldn't have filed the PR. Strange...

@constantinpape
Copy link
Owner

FWIW I just tried locally and the tests also fail for me.
I am using python 3.9 with the following mrcfile:

mrcfile                   1.3.0              pyh44b312d_0    conda-forge

@martinschorb
Copy link
Contributor Author

It seems we haven't included the axis juggling...? that would explain mobie/clem-example-project#20

@martinschorb
Copy link
Contributor Author

???

$ python io_tests/test_mrc_wrapper.py 
....
----------------------------------------------------------------------
Ran 4 tests in 0.497s

OK

with:

$ python -c 'import mrcfile; print(mrcfile.version.__version__)'
1.3.0

@constantinpape
Copy link
Owner

???

$ python io_tests/test_mrc_wrapper.py 
....
----------------------------------------------------------------------
Ran 4 tests in 0.497s

OK

with:

$ python -c 'import mrcfile; print(mrcfile.version.__version__)'
1.3.0

The tests pass, so I am not sure what's confusing you here.

@constantinpape
Copy link
Owner

Can you please also add a test for 2d data?

@martinschorb
Copy link
Contributor Author

So you just haven't merged it yet? I thought we got stuck with some errors in the tests. That was already too long ago and I was on holidays in between, so forgot about the details.
Then I will test the conversion for the 2D overview EM using my code and see how it looks...

@constantinpape
Copy link
Owner

So you just haven't merged it yet? I thought we got stuck with some errors in the tests. That was already too long ago and I was on holidays in between, so forgot about the details.
Then I will test the conversion for the 2D overview EM using my code and see how it looks...

I see. You're right. I also forgot about the details over that time. The tests are still failing in the CI...
I will check later if it works for me locally or not.

@constantinpape
Copy link
Owner

I can reproduce the test failures locally:

$ python test_mrc_wrapper.py 
FFFF
======================================================================
FAIL: test_dataset (__main__.TestMrcWrapper)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pape/Work/my_projects/elf/test/io_tests/test_mrc_wrapper.py", line 49, in test_dataset
    self.check_dataset(ds)
  File "/home/pape/Work/my_projects/elf/test/io_tests/test_mrc_wrapper.py", line 43, in check_dataset
    self.assertTrue(np.array_equal(out, exp))
AssertionError: False is not true

======================================================================
FAIL: test_dataset_compressed (__main__.TestMrcWrapper)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pape/Work/my_projects/elf/test/io_tests/test_mrc_wrapper.py", line 55, in test_dataset_compressed
    self.check_dataset(ds)
  File "/home/pape/Work/my_projects/elf/test/io_tests/test_mrc_wrapper.py", line 43, in check_dataset
    self.assertTrue(np.array_equal(out, exp))
AssertionError: False is not true

======================================================================
FAIL: test_file (__main__.TestMrcWrapper)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pape/Work/my_projects/elf/test/io_tests/test_mrc_wrapper.py", line 61, in test_file
    self.check_dataset(ds)
  File "/home/pape/Work/my_projects/elf/test/io_tests/test_mrc_wrapper.py", line 43, in check_dataset
    self.assertTrue(np.array_equal(out, exp))
AssertionError: False is not true

======================================================================
FAIL: test_file_compressed (__main__.TestMrcWrapper)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pape/Work/my_projects/elf/test/io_tests/test_mrc_wrapper.py", line 67, in test_file_compressed
    self.check_dataset(ds)
  File "/home/pape/Work/my_projects/elf/test/io_tests/test_mrc_wrapper.py", line 43, in check_dataset
    self.assertTrue(np.array_equal(out, exp))
AssertionError: False is not true

----------------------------------------------------------------------
Ran 4 tests in 0.286s

FAILED (failures=4)

I also compared a few values quickly and they are very different, this is not just some rounding error.

And I have the same mrcfile version that you have:

$ python -c "import mrcfile; print(mrcfile.__version__)"
1.3.0

@martinschorb
Copy link
Contributor Author

confirm. Set up anew env and now it fails for me as well. Let me explore what's going on...

@martinschorb
Copy link
Contributor Author

OK, I know what the problem is. Obviously the mrcfile writer handles the data the same as the reader. Therefore the axis order is consistent without the changes. But the order still needs to be swapped! So I changed that when generating the test data. Also it is working for both 2D and 3D data now with the new dimensionless code, so I think a 3D test should be sufficient.

@martinschorb
Copy link
Contributor Author

so locally the tests run now for me:

$ python -c 'import mrcfile; print(mrcfile.version.__version__)'
1.3.0
$ python test_mrc_wrapper.py 
....
----------------------------------------------------------------------
Ran 4 tests in 0.686s

OK

Copy link
Owner

@constantinpape constantinpape left a comment

Choose a reason for hiding this comment

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

The tests pass in the CI as well now. Thanks!

@constantinpape constantinpape merged commit d1c028b into constantinpape:master Apr 29, 2022
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.

2 participants