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

Integrate QC report generation into the workflow #40

Merged
merged 22 commits into from
Jul 17, 2024
Merged

Conversation

BenGros
Copy link
Contributor

@BenGros BenGros commented Jun 14, 2024

QC reports are generated for flat-field correction, whole slice viewing and a 3D volume rendered model. These reports are accessible through a locally run HTML file. Also added a script for producing test datasets that are subsets of an original dataset.

The ff correction and whole slice viewing use locally stored images that are created.
The 3D viewer generates a JSON file containing the data and generates the model from that.

@akhanf
Copy link
Member

akhanf commented Jun 21, 2024

I added the commits I made to fix some things and an example of splitting off the rule..

@Karl5766
Copy link

Karl5766 commented Jul 12, 2024

Code looks good to me, but some documentation could be better. Can you add documentation to briefly describe how to run the .js files? In docs/ or/and using docstring """ """ at the top of the file (as in PEP 257) would be fine.

Also may I ask what's the purpose of the dataset created by testing/create_test_dataset.py? I was looking around but not sure what it's used in. I think it may be good to add a description of this as docstrings as well.

Other .py files can benefit from docstrings as well but I think these two will be most helpful.

Edit: Just saw the issue to add documentation - if it's not part of the PR and will be done in the future then I believe we can merge the PR now.

@BenGros
Copy link
Contributor Author

BenGros commented Jul 15, 2024

I can add the documentation for running the js files in this pull request. And the create_test_dataset.py is used to create a subset of a large raw dataset to be able to test the workflow quickly.

Copy link
Member

@akhanf akhanf left a comment

Choose a reason for hiding this comment

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

Looks good to me!

workflow/rules/flatfield_corr.smk Outdated Show resolved Hide resolved
workflow/rules/ome_zarr.smk Outdated Show resolved Hide resolved
workflow/rules/ome_zarr.smk Outdated Show resolved Hide resolved
workflow/scripts/create_viewer.py Outdated Show resolved Hide resolved
shutil.move(resource_dir / "volRender.html", html_dest)

# Get most downsampled ome-zarr image
ds_z = ZarrNii.from_path(ome_data,level=5, channels=[0,1])
Copy link
Member

Choose a reason for hiding this comment

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

hard-coding the channels here fails when we have a different number of channels, but I think the best solution is to make a fix in zarrnii so that the default channels selects all of them (instead of just channel 0).. I'll make a note there to fix it, then we can change this line in a future PR..

akhanf added a commit that referenced this pull request Jul 17, 2024
needed because of basicpy update
@akhanf akhanf added the enhancement New feature or request label Jul 17, 2024
@akhanf
Copy link
Member

akhanf commented Jul 17, 2024

@BenGros this looks good to merge now, when you're ready you can do a Squash and Merge to clean the history, and provide a commit message and summarizes the PR.

@BenGros BenGros merged commit dbefbf6 into khanlab:main Jul 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants