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.

Sign up for GitHub

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

Jump to bottom

feat: Expand the Tomgram importer hash function to include processing software #340

Merged
merged 5 commits into from
Nov 5, 2024
Merged

feat: Expand the Tomgram importer hash function to include processing software #340

merged 5 commits into from
Nov 5, 2024

Conversation

uermel
Copy link
Contributor

@uermel uermel commented Nov 4, 2024 •
edited
Loading

Relates to N/A

Description

Tomograms are considered unique based on these fields during S3 ingestion:

[
                container_key,
                str(voxel_spacing),
                metadata.get("alignment_metadata_path", kwargs.get("alignment_metadata_path", "")),
                metadata.get("reconstruction_method", ""),
                metadata.get("processing", ""),
                str(metadata.get("deposition_id", int(parents["deposition"].name))),
]

This PR adds processing_software to the hash function to be able to distinguish tomograms denoised using different tools. It also expands the ID fields for the TomogramDBImporter.

@uermel uermel requested review from manasaV3 and jgadling November 4, 2024 19:59
@jgadling
Copy link
Contributor

jgadling commented Nov 4, 2024

I'll need to make the equivalent change to the v2 api ingestion as well. I'll add a commit to this PR shortly.

jgadling
jgadling reviewed Nov 4, 2024
View reviewed changes
apiv2/db_import/importers/tomogram.py Outdated
# id_fields = ["run_id", "tomogram_voxel_spacing_id", "deposition_id", "alignment_id", "processing", "reconstruction_method""]
id_fields = ["run_id", "tomogram_voxel_spacing_id", "deposition_id", "processing", "reconstruction_method"]
id_fields = [
"alignment_id",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure we can add alignment_id here yet, let me check on that!

Copy link
Contributor

Choose a reason for hiding this comment

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

I can revert it to be commented for now..

@jgadling jgadling merged commit 3a92596 into main Nov 5, 2024
8 checks passed
@jgadling jgadling deleted the uermel/tomogram_hash branch November 5, 2024 14:54
@czi-github-helper czi-github-helper bot mentioned this pull request Nov 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgadling jgadling jgadling approved these changes

@manasaV3 manasaV3 Awaiting requested review from manasaV3

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@uermel @jgadling @manasaV3