-
Notifications
You must be signed in to change notification settings - Fork 9
Extend MRC to Nexus converter to work with TVIPS #300
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
Conversation
Extend ED_mrc_to_nexus command to work with TVIPS cameras (besides eBIC Ceta-D detector). The command now has an option --detector that could be set either to cetad or tvips.
noemifrisina
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, thanks. Definitely much tidier with one command line tool.
I added a couple of comments but nothing big.
Please rebase on top of main before merging - although there shouldn't be any conflicts.
Also, a lot of bits in the code look to be unformatted, could you please run pre-commits on them?
| parser.add_argument( | ||
| "--detector", | ||
| type=str, | ||
| default="cetad", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add a choices=["cetad", "tvips"] option to the argument so that it will accept only those values and show them as part of the help.
src/nexgen/tools/mrc_tools.py
Outdated
| return n, out_file, np.array(angles) | ||
| else: | ||
| msg = "The converter expects either a list of MRC images\n" | ||
| msg = "or a single MRC file with a stack of images." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is complaining because the next msg is missing a '+'
src/nexgen/tools/mrc_tools.py
Outdated
| import numpy as np | ||
|
|
||
|
|
||
| def cal_wavelength(V0): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could: add some typing to these functions?
Also it would be good to have a couple of tests...
Bumps [h5py](https://github.com/h5py/h5py) from 3.11.0 to 3.13.0. - [Release notes](https://github.com/h5py/h5py/releases) - [Changelog](https://github.com/h5py/h5py/blob/master/docs/release_guide.rst) - [Commits](h5py/h5py@3.11.0...3.13.0) --- updated-dependencies: - dependency-name: h5py dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Noemi Frisina <54588199+noemifrisina@users.noreply.github.com>
Bumps [pydantic](https://github.com/pydantic/pydantic) from 2.10.3 to 2.11.4. - [Release notes](https://github.com/pydantic/pydantic/releases) - [Changelog](https://github.com/pydantic/pydantic/blob/main/HISTORY.md) - [Commits](pydantic/pydantic@v2.10.3...v2.11.4) --- updated-dependencies: - dependency-name: pydantic dependency-version: 2.11.4 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [numpy](https://github.com/numpy/numpy) from 2.2.2 to 2.2.6. - [Release notes](https://github.com/numpy/numpy/releases) - [Changelog](https://github.com/numpy/numpy/blob/main/doc/RELEASE_WALKTHROUGH.rst) - [Commits](numpy/numpy@v2.2.2...v2.2.6) --- updated-dependencies: - dependency-name: numpy dependency-version: 2.2.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Update pydantic things * A start and some notes * Replace old namedtuples * Start rethinking parameter model * Add utility function to be used by nexgen server for serial * Update changelog * Fix test * Add a couple of dumb tests * Tidy up where possible * For now comment out before refactoring * Pass timestamps as datetime * Replace deprecated function * Fix iso timestamp bug and add test * Improve validation * Small test detail * Small improvements on general writer * Tidy ups * Update changelog * Start adding to CLI to make it nicer * Update command line for i19 * Update changelog * Add docstring * Fix typo
Extend ED_mrc_to_nexus command to work with TVIPS cameras (besides eBIC Ceta-D detector). The command now has an option --detector that could be set either to cetad or tvips.