Skip to content
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

Calibration Unit #10

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

Conversation

ThorstenFalk
Copy link

  • Respect Calibration Unit for element_size_um computation
  • For 2D datasets element_size_um might be stored as 2-element-vector leading to "Array out of bounds" exceptions. This case is now detected and a one prepended.

- Respect Calibration Unit for element_size_um computation
- For 2D datasets element_size_um might be stored as 2-element-vector leading to "Array out of bounds" exceptions. This case is now detected and a one prepended in this case.
@ThorstenFalk
Copy link
Author

I currently maintain my own version of the HDF5_Vibez plugin on the U-Net Segmentation site which includes the proposed patch. Currently the plugin simply assumes that Calibration is in micrometers, the patch should catch at least all metric units and scale the values accordingly. Feel free to also add inch or other unit names respected by Fiji.

Copy link
Collaborator

@rejsmont rejsmont left a comment

Choose a reason for hiding this comment

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

Hi,

I will review and test these within a week. Please ping me back if I forget for any reason. From a quick look, it looks good.

Do you have other changes from your tree? I guess we should merge these two repos...

R.

@ThorstenFalk
Copy link
Author

My tree is based on the original version from Olaf, so I think we used the same code base. Besides some newline and whitespace changes my tree is just behind your version and with the merge mine becomes obsolete.

@ThorstenFalk
Copy link
Author

Ah, I just realized, the getElementSizeUm(imp) method will return the element size as 2-element vector if the hyperstack has only on spatial slice. With the proposed changes in the load methods, this should be no issue, but for backward-compatibility you may want to change this to a three-element vector.

@rejsmont
Copy link
Collaborator

Hi,

I am terribly sorry for the delay, it has been crazy setting up my lab. Could you please re-merge your branch with master and I will take a look at the code again.

Thanks,

R.

Copy link
Collaborator

@rejsmont rejsmont left a comment

Choose a reason for hiding this comment

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

The changes look good to me. As soon as you merge your branch with the master we should be good to merge. If you have time, unit tests for this would be great...

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