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

IGVF-2113-index-file #1206

Merged
merged 10 commits into from
Nov 20, 2024
Merged

IGVF-2113-index-file #1206

merged 10 commits into from
Nov 20, 2024

Conversation

jenjou
Copy link
Contributor

@jenjou jenjou commented Nov 18, 2024

No description provided.

"audit_description": "Alignment files in bai format are expected to have their corresponding bam file in `derived_from`.",
"audit_category": "incorrect bam file",
"audit_description": "Index files in tbi format are expected to have a corresponding tsv or vcf file in `derived_from`.",
"audit_category": "incorrect indexed file",
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be "missing indexed file"?

Copy link
Contributor

Choose a reason for hiding this comment

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

nevermind, I just realized derived_from is required for index files. But for consistency with the wording for other audits I think the category should be "unexpected indexed file" instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW you should add this file type to the list of audit scripts in commands/make_audit_docstring.py

Comment on lines +107 to +110
"enum": [
"index"
],
Copy link
Contributor

Choose a reason for hiding this comment

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

can you provide enum_descriptions?

"$merge": [
"file.json#/dependentSchemas"
],
"file_format": {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you leave comments for these dependencies

Copy link
Contributor

@ian-whaling ian-whaling left a comment

Choose a reason for hiding this comment

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

some minor comments, looks pretty good overall other than that.

ian-whaling
ian-whaling previously approved these changes Nov 19, 2024
@jenjou jenjou force-pushed the IGVF-2113-index-file branch from b6e629f to c494486 Compare November 20, 2024 01:34
@ottojolanki ottojolanki merged commit 9ed349d into dev Nov 20, 2024
9 checks passed
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