-
Notifications
You must be signed in to change notification settings - Fork 11
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: correct alignment id in download modal #1477
base: main
Are you sure you want to change the base?
Conversation
* main: (25 commits) feat: Diff logging for deposition page (#1467) chore(main): release web 1.43.0 (#1474) feat: Another dummy PR (#1473) chore(main): release web 1.42.0 (#1472) feat: Dummy PR to trigger release please (#1471) chore(main): release web 1.41.0 (#1461) feat: Update ML Challenge banner (#1457) feat: Convert deposition page query to V2 (#1452) fix: Fix deposition filter on run page (#1466) docs: Add anchors to FAQ questions (#1458) chore: Refactor datasets filter values fragment (#1456) feat: Add plausible analytics tracking to pages (#1459) chore(main): release web 1.40.0 (#1441) feat: Add diff detection for new datasets page query (#1446) fix: Remove non-null assertions in dataset page hook (#1450) fix: Never send empty argument objects to APIv2 (#1447) fix: Fix deposition dropdown filters being broken (#1448) feat: Migrate datasets page query (#1438) test: filter components unit tests (#1436) chore(main): release web 1.39.0 (#1437) ...
(tomogram) => tomogram.isPortalStandard, | ||
referenceTomogramId: ( | ||
tomograms.find((tomogram) => tomogram.isPortalStandard) ?? | ||
tomograms.at(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget, is every run guaranteed to have a portal standard tomogram (if it has any at all)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no idea, @uermel do you know?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not all of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it's possible for a run to have non-zero number of tomograms, and none of them are portal standard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, yes. And I think this will remain the case for a small number of runs even when standard tomograms are ingested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! So if a user is downloading an annotation from a run and there is no portal standard tomogram in that run, should we not show "Alignment ID: AL-XXXXX" in the download modal right? (actually, if there's no portal standard tomogram does that also mean there's no annotations? 🤔)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be annotations and I think there would be dummy alignments for these tomograms, even though there is no standard I believe. @manasaV3 can you confirm that all tomograms have a dummy alignment now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized, this is the modal for downloading an annotation. It should be showing the alignment ID of the annotationFile
that's being selected by the file type right? Not a tomogram?
closes #1373
before:
after: