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.
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 |
orbeckst
left a comment
orbeckst
left a comment
There was a problem hiding this comment.
Sorry it took me a while to get back to the PR. Thank you for addressing all remaining issues and improving the tests. This is good to go and I'll merge.
|
@marinegor could you please have a quick look and confirm that all your concerns have been addressed? Thanks! |
3 participants
Fix Issue - #108
Add MRC file writing support