-
Notifications
You must be signed in to change notification settings - Fork 1
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
base: develop
Are you sure you want to change the base?
Conversation
@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. |
4ba7a19
to
5e534e9
Compare
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 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.
@nyoungbq Thanks for the feedback. |
5165389
to
94d70bc
Compare
@nyoungbq I'm not going to change anyting in the "oless" folder since that actually came from another GitHub repo. |
@nyoungbq Removed the temp DataStructure. Calculated the feedback strings inline. Fixed other bugs related to the slice start and stop values. |
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.
@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
94d70bc
to
c5a0c91
Compare
@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 |
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.
Good to merge
6968767
to
d45782d
Compare
d45782d
to
d717717
Compare
d717717
to
2f25d65
Compare
2cd344a
to
6c43bea
Compare
2f25d65
to
2f3d6be
Compare
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
2f3d6be
to
3b5c175
Compare
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.