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

Fixed previous fixed error for tests #118

Merged
merged 1 commit into from
Aug 14, 2023
Merged

Fixed previous fixed error for tests #118

merged 1 commit into from
Aug 14, 2023

Conversation

Pablo1990
Copy link
Contributor

@Pablo1990 Pablo1990 commented Jul 31, 2023

I commit this same fix, but don't know why it didn't go through (even though I pushed it)...

@giuliapaci
Copy link
Collaborator

Hi Pablo,

I had made that change (adding a _) previously to fix a problem which occurs now if you try running the 1-channel projection. This is because calculate_projection now gives two outputs so the _ captures the "empty" one which is a None in the case of a single channel.

To replicate the error (attached), I opened a stack in Napari and then tried running Plugins -> epitools -> Projection(selective plane)

If you look in the tests, inside test_projection, test_calculate_projection has the same _ and that test passes. If you remove it, it fails. Not sure how to fix the linter without running again into this problem

Screenshot 2023-08-11 at 09 23 39

@Pablo1990
Copy link
Contributor Author

Hi Giulia,

Thanks for checking! We might have to use two different functions then or maybe and if-else? What do you think it would be better?

@alessandrofelder
Copy link
Collaborator

alessandrofelder commented Aug 11, 2023

Not sure it's much nicer, but you could use some enumerating to handle both cases?
Something like (pseudocode for main.py):

viewer = napari.current_viewer()
projected_data = calculate_projections(...) # put all outputs into the same variable irrespective of how many there are
for i, projection in enumerate(projected_data):
		if projection is not None:
	        viewer.add_image(
	            data=projection,
	            name=f"Projection_ch{i}",
	            scale=image.scale,
	            translate=image.translate,
	            rotate=image.rotate,
	            plane=image.plane,
	            metadata=image.metadata,
	        ) 

@Pablo1990 Pablo1990 reopened this Aug 11, 2023
@Pablo1990
Copy link
Contributor Author

I don't know why it closed, I just reset the branch. Now, I've fixed the tests rather than touching the code. Tests are passing now.

Copy link
Collaborator

@giuliapaci giuliapaci left a comment

Choose a reason for hiding this comment

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

All looks good, I have tested with images and both projection (1 channel and 2 channel) work!

@Pablo1990 Pablo1990 merged commit 94cb621 into main Aug 14, 2023
11 checks passed
@Pablo1990 Pablo1990 deleted the MissingCommit branch August 14, 2023 08:10
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.

None yet

3 participants