Skip to content

FILT: Read Zeiss TXM/TXRM xCT files #2

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

imikejackson
Copy link
Contributor

@imikejackson imikejackson commented Mar 10, 2025

Unit Test: We need to find or acquire a small txm file for testing. This is holding this filter back from being included in SIMPLNXCore plugin.

@imikejackson imikejackson requested a review from nyoungbq March 10, 2025 13:31
@imikejackson
Copy link
Contributor Author

@nyoungbq The code is still in a bit of rough shape but the basics are there. I need a unit test and better docs. Since the files are so large not sure where I can come up with a small sample file.

@imikejackson imikejackson force-pushed the filter/read_txm_file branch 3 times, most recently from 4ba7a19 to 5e534e9 Compare March 10, 2025 20:59
Copy link

@nyoungbq nyoungbq left a comment

Choose a reason for hiding this comment

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

I am approving it since I didn't see any obvious bugs, and this repo is more lax in requirements since it's a staging ground.

For future changes: I poured over it, there were a couple of things that stood out, such as making a temporary datastructure/image geom to calculate the origin (should just be done with a util function or inlined calculations), commented out code, and logic paths that do nothing.

@imikejackson
Copy link
Contributor Author

@nyoungbq Thanks for the feedback.

@imikejackson imikejackson force-pushed the filter/read_txm_file branch 3 times, most recently from 5165389 to 94d70bc Compare March 11, 2025 21:38
@imikejackson
Copy link
Contributor Author

@nyoungbq I'm not going to change anyting in the "oless" folder since that actually came from another GitHub repo.

@imikejackson
Copy link
Contributor Author

@nyoungbq Removed the temp DataStructure. Calculated the feedback strings inline. Fixed other bugs related to the slice start and stop values.

Copy link

@nyoungbq nyoungbq left a comment

Choose a reason for hiding this comment

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

@imikejackson looked it over the files that changed and it all looks good, the two offered changes are little and just give the compiler more chances to optimize

@imikejackson imikejackson force-pushed the filter/read_txm_file branch from 94d70bc to c5a0c91 Compare March 12, 2025 12:26
@imikejackson
Copy link
Contributor Author

@nyoungbq Used a swtich statement instead to make it clear we are only using values from the enumeration.

@nyoungbq
Copy link

@nyoungbq Used a swtich statement instead to make it clear we are only using values from the enumeration.

That works, chances are it will be translated to the same thing either way. Comparison is faster for small cases but most of the time the llvm will optimize accordingly under the covers. Here's an overflow post if you are interested: https://stackoverflow.com/a/60110730

Copy link

@nyoungbq nyoungbq left a comment

Choose a reason for hiding this comment

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

Good to merge

@imikejackson imikejackson force-pushed the filter/read_txm_file branch 3 times, most recently from 6968767 to d45782d Compare March 19, 2025 19:39
@imikejackson imikejackson force-pushed the filter/read_txm_file branch from d45782d to d717717 Compare March 22, 2025 19:59
@imikejackson imikejackson force-pushed the filter/read_txm_file branch from d717717 to 2f25d65 Compare April 1, 2025 12:32
@imikejackson imikejackson force-pushed the develop branch 2 times, most recently from 2cd344a to 6c43bea Compare April 3, 2025 00:07
@imikejackson imikejackson force-pushed the filter/read_txm_file branch from 2f25d65 to 2f3d6be Compare April 3, 2025 00:08
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
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