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

Fix points_to_spheres bug in 3D #932

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

heinsimon
Copy link
Contributor

Closes #917

@ma-sadeghi ma-sadeghi changed the title Closes #917: points_to_spheres should now adds spheres in 3D correctly Fix points_to_spheres bug in 3D Mar 13, 2024
Copy link
Member

@ma-sadeghi ma-sadeghi left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Could you please also add a unit test to test 2D vs. 3D behaviour?

@heinsimon
Copy link
Contributor Author

I added a simple test case for 2D and 3D.

I did not add anything for im being a boolean array, since I do not really know what the expected output would be. It feels broken, but I do not know for sure.

@ma-sadeghi
Copy link
Member

I added a simple test case for 2D and 3D.

I did not add anything for im being a boolean array, since I do not really know what the expected output would be. It feels broken, but I do not know for sure.

That seems strange, I can't look at it right now, but will do later this week.

@heinsimon
Copy link
Contributor Author

Is this PR still considered or is it outdated now?

@ma-sadeghi
Copy link
Member

Sorry for the delay. I still didn't get the chance to review it. Will do this week hopefully.

@jgostick
Copy link
Member

jgostick commented Jun 4, 2024

I'm so sorry how long this is taking. Amin has a new job now, so it's all on me at the moment. I think the PR is pretty much ready, I'll just go in and fix the tests myself asap.

@jgostick
Copy link
Member

jgostick commented Jun 5, 2024

I see that "maintainers are allowed to edit this pull request" is enabled, but I honestly don't know how. I would have expected to see this as a branch on my remote which I could pull locally to work on, but that is not how it works. Soooo, I'm just going to merge it into dev, then create a new PR to fix the tests.

@jgostick jgostick merged commit 3c8f172 into PMEAL:dev Jun 5, 2024
7 checks passed
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.

points_to_spheres not adding spheres correctly for 3D images
3 participants