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

Display biolucida images in gallery based on scicrunch #152

Merged
merged 4 commits into from
Jul 10, 2024

Conversation

ddjnw1yu
Copy link
Collaborator

@ddjnw1yu ddjnw1yu commented Jul 3, 2024

For sprint ticket - Bug Submission: Image viewer not opening

This should help to hide the biolucida image whose id does not exist in Scicrunch.

Copy link

cypress bot commented Jul 3, 2024

Passing run #148 ↗︎

0 77 1 0 Flakiness 0

Details:

Use Scicrunch data to create thumbnail gallery items
Project: SPARC Vue3 Portal Testing Commit: 5529917740
Status: Passed Duration: 17:38 💡
Started: Jul 9, 2024 11:20 AM Ended: Jul 9, 2024 11:38 AM

Review all test suite changes for PR #152 ↗︎

@ddjnw1yu ddjnw1yu changed the title Filter biolucida images based on scicrunch Display biolucida images in gallery based on scicrunch Jul 3, 2024
@ddjnw1yu ddjnw1yu requested review from alan-wu and egauzens July 3, 2024 00:28
Copy link
Collaborator

@egauzens egauzens left a comment

Choose a reason for hiding this comment

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

I don't think this is right since there does appear to be images for those gallery items, for example:

https://sparc.science/datasets/file/287/1?path=files/derivative/MicroCT-and-Segmentation/sub-pig-2/Nerve-2-Segmentation-Files/22_2.jpx

@egauzens
Copy link
Collaborator

egauzens commented Jul 3, 2024

I don't think this is right since there does appear to be images for those gallery items, for example:

https://sparc.science/datasets/file/287/1?path=files/derivative/MicroCT-and-Segmentation/sub-pig-2/Nerve-2-Segmentation-Files/22_2.jpx

Ah nvm I just read the comments you posted on the Wrike ticket. So ideally once scicrunch has been updated then the images would show again in the gallery, correct?

@egauzens egauzens self-requested a review July 3, 2024 17:15
@egauzens egauzens dismissed their stale review July 3, 2024 17:15

Saw updated wrike comments

@ddjnw1yu
Copy link
Collaborator Author

ddjnw1yu commented Jul 3, 2024

I don't think this is right since there does appear to be images for those gallery items, for example:
https://sparc.science/datasets/file/287/1?path=files/derivative/MicroCT-and-Segmentation/sub-pig-2/Nerve-2-Segmentation-Files/22_2.jpx

Ah nvm I just read the comments you posted on the Wrike ticket. So ideally once scicrunch has been updated then the images would show again in the gallery, correct?

Yes, that's right. As long as related Biolucida metadata is updated in Scicrunch, images will show again. The function still uses the Biolucida data to generate gallery image items. Just added one more filtering step to check if these images exist on Scicrunch.

@alan-wu
Copy link
Contributor

alan-wu commented Jul 4, 2024

@ddjnw1yu A more complete solution is to use the Scicrunch response itself to build up all the biolucida image items on the gallery instead of making an extra call with biolucida.searchDataset. It will be similar to theway scaffold, flatmap and plot item are populate but only include biolucida2d/biolucida3d items with a biolucida id.

@alan-wu
Copy link
Contributor

alan-wu commented Jul 4, 2024

@egauzens FYI, some datasets have a lot of biolucida images and not all of them are meant to present on the gallery so the presence of a biolucida id gives us an idea of whether a biolucida images should be present/highlighted on the gallery.

@ddjnw1yu
Copy link
Collaborator Author

ddjnw1yu commented Jul 4, 2024

@ddjnw1yu A more complete solution is to use the Scicrunch response itself to build up all the biolucida image items on the gallery instead of making an extra call with biolucida.searchDataset. It will be similar to theway scaffold, flatmap and plot item are populate but only include biolucida2d/biolucida3d items with a biolucida id.

I have taken a look at the code and had a try. We probably no need to do this. From what I saw, we will still need to call biolucida.searchDataset because some required information does not exist on Scicrunch. Sometimes, Scicrunch may have hundreds of biolucida objects (less on biolucida server).

@ddjnw1yu
Copy link
Collaborator Author

ddjnw1yu commented Jul 9, 2024

The latest commit uses Scicrunch data to create gallery items.

Copy link
Collaborator

@egauzens egauzens left a comment

Choose a reason for hiding this comment

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

I'm fine with either way we find the gallery images, it does look cleaner just getting from scicrunch and so long as it seems about the same computationally then I'm ok with this

@egauzens egauzens merged commit d42e0a6 into nih-sparc:main Jul 10, 2024
2 checks passed
@ddjnw1yu ddjnw1yu deleted the hide-biolucida-if-not-in-scicrunch branch July 11, 2024 21:21
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.

3 participants