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

Fix breaking changes introduced in the NCBI API #102

Merged
merged 6 commits into from
Jul 1, 2022
Merged

Fix breaking changes introduced in the NCBI API #102

merged 6 commits into from
Jul 1, 2022

Conversation

drpatelh
Copy link
Member

@drpatelh drpatelh commented Jul 1, 2022 •
edited
Loading

See #99

NCBI recently updated their base API URLs which broke the functionality in this pipeline to fetch metadata for SRA and GEO ids.

I can't find a way to obtain GEO metadata using the current APIs so I will strip this functionality out of the pipeline and add it back in when a robust solution exists.

✅ Fixed for SRA ids

I have fixed the base URL for fetching SRA metadata based on the docs here

Old URL

https://trace.ncbi.nlm.nih.gov/Traces/sra/sra.cgi?save=efetch&db=sra&rettype=runinfo&term=SRR14593545

New URL

https://eutils.ncbi.nlm.nih.gov/entrez/eutils/efetch.fcgi?id=SRR14593545&db=sra&rettype=runinfo&retmode=text

❌ Broken for GEO ids

I tried to update the base URL for fetching GEO metadata based on the docs here but it is still broken....

Old URL

https://www.ncbi.nlm.nih.gov/geo/query/acc.cgi?acc=GSM4907283&targ=gsm&view=data&form=text

New URL

https://eutils.ncbi.nlm.nih.gov/entrez/eutils/efetch.fcgi?id=GSM4907283&db=gds&rettype=runinfo&retmode=text

@github-actions
Copy link

github-actions bot commented Jul 1, 2022 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 3696f40

+| ✅ 143 tests passed       |+
#| ❔   2 tests were ignored |#
!| ❗   4 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: conf/igenomes.config
  • files_exist - File not found: lib/WorkflowFetchngs.groovy
  • nextflow_config - Config manifest.version should end in dev: '1.7'
  • readme - README did not have a Nextflow minimum version badge.

❔ Tests ignored:

  • files_unchanged - File ignored due to lint config: assets/sendmail_template.txt
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy

✅ Tests passed:

Run details

  • nf-core/tools version 2.4.1
  • Run at 2022-07-01 15:36:40

maxulysse
maxulysse reviewed Jul 1, 2022
View reviewed changes
assets/schema_input.json Outdated Show resolved Hide resolved
@drpatelh drpatelh merged commit 80bdaa8 into nf-core:dev Jul 1, 2022
@drpatelh drpatelh mentioned this pull request Jul 1, 2022
Closed
@hukai916
Copy link

To fetch metadata given GEO id, will ffq help: https://github.com/pachterlab/ffq ? Seems that ffq calls esearch utilities first and parse the results to obtain a full meta table.

@drpatelh
Copy link
Member Author

Hi @hukai916 ! Yep, I have already created a draft PR to add in a ffq implementation #100 and just created an issue to officially track this feature #110

@adf-ncgr adf-ncgr mentioned this pull request Oct 13, 2022
Open
@drpatelh drpatelh modified the milestones: 1.8, 1.9 Nov 4, 2022
@Midnighter
Copy link
Contributor

Using esearch first also works here:

  1. https://eutils.ncbi.nlm.nih.gov/entrez/eutils/esearch.fcgi?db=sra&term=GSM4907283
  2. https://eutils.ncbi.nlm.nih.gov/entrez/eutils/efetch.fcgi?db=sra&id=12420761&rettype=runinfo&retmode=text

@ejseqera
Copy link
Contributor

ejseqera commented May 9, 2023

An eutils workflow for working with GEO IDs is somewhat described here: https://www.ncbi.nlm.nih.gov/geo/info/geo_paccess.html (updated March 27 2023)

@ejseqera ejseqera assigned ejseqera and unassigned ejseqera May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxulysse maxulysse maxulysse approved these changes

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@drpatelh @hukai916 @Midnighter @ejseqera @maxulysse