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

feat: Check other endpoints for ATAC artifacts #7442

Merged
merged 6 commits into from
Feb 20, 2025

Conversation

ivirshup
Copy link
Contributor

@ivirshup ivirshup commented Feb 19, 2025

Reason for Change

Changes

  • added FRAGMENT_INDEX artifact type
  • modified permanent url resolution code to account for URI for fragment indices
  • added tests checking that atac artifacts are returned by the API

Notes for reviewer

In the specification doc there is some inconsistency around what the index artifact is called. I went with FRAGMENT_INDEX but FRAGMENT_TABIX is also used in the spec.

@ivirshup ivirshup changed the base branch from main to feature/schema-5-3 February 19, 2025 00:03
@ivirshup ivirshup changed the title Feat: Check other endpoints for ATAC artifacts feat: Check other endpoints for ATAC artifacts Feb 19, 2025
@ivirshup ivirshup marked this pull request as ready for review February 19, 2025 00:16
@ivirshup ivirshup requested a review from Bento007 February 19, 2025 00:17
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (feature/schema-5-3@77d0000). Learn more about missing BASE report.

Additional details and impacted files
@@                  Coverage Diff                  @@
##             feature/schema-5-3    #7442   +/-   ##
=====================================================
  Coverage                      ?   92.99%           
=====================================================
  Files                         ?      196           
  Lines                         ?    16721           
  Branches                      ?        0           
=====================================================
  Hits                          ?    15550           
  Misses                        ?     1171           
  Partials                      ?        0           
Flag Coverage Δ
unittests 92.99% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Bento007 Bento007 left a comment

Choose a reason for hiding this comment

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

A few minor things to fix

@@ -818,6 +818,8 @@ components:
enum:
- H5AD
- RDS
- FRAGMENT_TSV
Copy link
Contributor

Choose a reason for hiding this comment

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

We landed on using ATAC_FRAGMENT, and ATAC_INDEX

ivirshup and others added 2 commits February 20, 2025 13:25
Co-authored-by: Trent Smith <1429913+Bento007@users.noreply.github.com>
@ivirshup ivirshup requested a review from Bento007 February 20, 2025 21:47
@ivirshup ivirshup merged commit 05f7743 into feature/schema-5-3 Feb 20, 2025
31 of 34 checks passed
@ivirshup ivirshup deleted the ivirshup/check-get-endpoints-for-atac branch February 20, 2025 22:25
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.

2 participants