Skip to content

Implementation of from_DOI() - #5454

Open
jauy123 wants to merge 112 commits into
MDAnalysis:developfrom
jauy123:Fetcher_DOI
Open

Implementation of from_DOI()#5454
jauy123 wants to merge 112 commits into
MDAnalysis:developfrom
jauy123:Fetcher_DOI

Conversation

@jauy123

@jauy123 jauy123 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #5434

Changes made in this Pull Request:

  • Implements from_DOI using StaticFetcher()

LLM / AI generated code disclosure

LLMs or other AI-powered tools (beyond simple IDE use cases) were used in this contribution: no

PR Checklist

  • [ x] Issue raised/referenced?
  • [ x] Tests updated/added?
  • Documentation updated/added?
  • package/CHANGELOG file updated?
  • Is your name in package/AUTHORS? (If it is not, add it!)
  • LLM/AI disclosure was updated.

Developers Certificate of Origin

I certify that I can submit this code contribution as described in the Developer Certificate of Origin, under the MDAnalysis LICENSE.

@read-the-docs-community

read-the-docs-community Bot commented Aug 9, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.59864% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.88%. Comparing base (83bc9a2) to head (9a269d4).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
package/MDAnalysis/fetch/fetchers.py 96.74% 3 Missing and 1 partial ⚠️
package/MDAnalysis/fetch/pdb.py 95.65% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5454      +/-   ##
===========================================
+ Coverage    93.87%   93.88%   +0.01%     
===========================================
  Files          182      183       +1     
  Lines        22510    22650     +140     
  Branches      3202     3228      +26     
===========================================
+ Hits         21131    21265     +134     
- Misses         917      921       +4     
- Partials       462      464       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jauy123

jauy123 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

This diverged from #5436 using StaticFetcher as a base.

@jauy123

jauy123 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

docs still need to be changed and updated

@jauy123 jauy123 changed the title Implementatiom of from_DOI() Implementation of from_DOI() Aug 12, 2026
@jauy123

jauy123 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

docs for from_DOI is done

@orbeckst orbeckst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Whatever I said on PR #5457 also applies for the common StaticFetcher code here. I assume that once one of these gets merged, the other gets updated accordingly.

More generally though it sounds to me that the original way we thought about organizing fetchers in sub-modules by format (or source) is not useful. The from_xxx() designates the source, the files argument determines the files, so the sub-module (namely, fetch.pdb) is not relevant or really confusing. Perhaps you already have a plan to reorganize everything once the current PRs are merged but if not, I'd suggest to just ditch fetch.pdb and make the fetch_xxx() functions directly available from MDAnalysis.fetch.

]

return paths if not isinstance(pdb_ids, str) else paths[0]
def from_DOI(doi, file_name, remove_prefix=True, cache_path=None):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's a bit confusing that this lives under fetch.pdb.from_doi() given that it can download any kind of files. The semantics are unclear because when I get a psf I do not "fetch a pdb-formatted file from a DOI".

Perhaps all these functions make more sense directly under fetch, at least fetch.from_doi(..., file="AdK.psf") does not spell a contradictory sentence.

@orbeckst orbeckst added GSoC GSoC project fetcher labels Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fetcher GSoC GSoC project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[GSoC 2026] Implementation of from_doi()

2 participants