Add MRC file writing support #147
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #147 +/- ##
==========================================
+ Coverage 87.70% 88.13% +0.43%
==========================================
Files 5 5
Lines 748 809 +61
Branches 97 105 +8
==========================================
+ Hits 656 713 +57
Misses 56 56
- Partials 36 40 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Looks good at first glance but I am not an MRC expert. I'll see if I can get someone else to also have a look.
In the meantime, please also
update your branch with the latest changes on master(just did that)- add yourself to AUTHORS
- add an entry to CHANGELOG
- check your coverage and add tests where necessary
gridData/mrc.py
Outdated
| header information (including mapc, mapr, maps ordering) is preserved. | ||
| Otherwise, standard ordering (mapc=1, mapr=2, maps=3) is used. | ||
| .. versionadded:: 0.8.0 |
There was a problem hiding this comment.
change to 1.1.0 (next minor release)
|
@marinegor do you have some experience with MRC files and would you be able to have a very brief look at this PR? |
marinegor
left a comment
marinegor
left a comment
There was a problem hiding this comment.
Hi, thanks for the PR!
I've added couple of comments regarding self.header vs self._mrc_header, and regarding using with open(...).
Also, perhaps it's a good idea to add a small test MRC (say, 4x4x4 with random numbers) file with/without a header that tests things? This way we can actually track regressions if something changes in mrcfile itself, while a roundtrip test won't check that.
| mrc_file = mrc.MRC() | ||
| mrc_file.array = self.grid | ||
| mrc_file.delta = numpy.diag(self.delta) | ||
| mrc_file.origin = self.origin | ||
| mrc_file.rank = 3 | ||
|
|
||
| # Transfer header if it exists (preserves axis ordering and other metadata) | ||
| if hasattr(self, '_mrc_header'): | ||
| mrc_file.header = self._mrc_header | ||
|
|
||
| # Write to file | ||
| mrc_file.write(filename) |
There was a problem hiding this comment.
I feel like this should be replaced with with mrcfile.new(filename) as mrc: ... block, no? Generally writing to files without using context manager is bad practice.
There was a problem hiding this comment.
_export_mrc() method uses MRC.write() which properly uses mrcfile.new( filename)
context manager is used -> in mrc.py line (194)
| # File was read - preserve original ordering | ||
| h = self.header |
There was a problem hiding this comment.
why is it self.header and not self._mrc_header as in core.py?
There was a problem hiding this comment.
core stores headers as _mrc_header . When exporting, we assign mrc_file.header = self._mrc_header, giving the MRC instance a header attribute. MRC.write() then finds self.header and uses it. its like different class different name.
There was a problem hiding this comment.
i will provide a test mrc as you mention in some time.
orbeckst
left a comment
orbeckst
left a comment
There was a problem hiding this comment.
Some minor fixes required and please address @marinegor 's comments. That should also help with test coverage.
CHANGELOG
Outdated
| * Added MRC file writing support (Issue #108) | ||
| * Implemented MRC.write() method with proper coordinate transformations | ||
| * Added Grid.export() integration for MRC format | ||
| * Preserved header information (mapc/mapr/maps, nstart, cell dimensions) | ||
| * Added comprehensive tests | ||
|
|
There was a problem hiding this comment.
Too much information, focus on user-facing information, just
| * Added MRC file writing support (Issue #108) | |
| * Implemented MRC.write() method with proper coordinate transformations | |
| * Added Grid.export() integration for MRC format | |
| * Preserved header information (mapc/mapr/maps, nstart, cell dimensions) | |
| * Added comprehensive tests | |
| * Added MRC file writing support (Issue #108) |
is sufficient.
CHANGELOG
Outdated
| 12/12/2025 Pradyumn-cloud | ||
| * 1.1.0 | ||
|
|
||
| Changes | ||
|
|
There was a problem hiding this comment.
All of your changes should go into the existing 1.1.0 section below. Leave the date as ??/??/???? and just
- add your handle after orbeckst
- add the one line "Added MRC file writing support (Issue Add support for writing MRC files #108)" in the section Enhancements
gridData/core.py
Outdated
| information (including axis ordering) is preserved | ||
| * For new grids, standard ordering (mapc=1, mapr=2, maps=3) is used | ||
| .. versionadded:: 0.8.0 |
3 participants
Fix Issue - #108
Add MRC file writing support