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

Add ASE ingester and generalize other ingestion utilities #1509

Merged
merged 6 commits into from Feb 14, 2023

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Feb 9, 2023

This PR adds from_ase as a new ingester type (following #1296 for pymatgen) and generalises the ingester functionality into a .ingest_from method of the base adapter, e.g.,

from optimade.adapters import Structure

atoms = ase.Atoms(...)
structure = pymatgen.core.Structure(...)

Structure.ingest_from(atoms)  # implicit type detection
Structure.ingest_from(structure)  # implicit type detection
Structure.ingest_from(atoms, "ase")  # use key into ingester dict to specify
Structure.ingest_from(pymatgen, "pymatgen")  # use key into ingester dict to specify

Also adds several utilities for e.g., normalizing formulae and other fields. These are now used in the adapters and spread across various utils modules (to avoid circular imports).

@ml-evs ml-evs added enhancement New feature or request adapters Issues pertaining to adapters (converters) ergonomics Features that improve the usability of the package labels Feb 9, 2023
…ests

- Update test error messages for new validator
@codecov
Copy link

codecov bot commented Feb 9, 2023

Codecov Report

Merging #1509 (baf4799) into master (7c8f9a6) will increase coverage by 0.06%.
The diff coverage is 94.38%.

@@            Coverage Diff             @@
##           master    #1509      +/-   ##
==========================================
+ Coverage   90.95%   91.01%   +0.06%     
==========================================
  Files          74       74              
  Lines        4411     4475      +64     
==========================================
+ Hits         4012     4073      +61     
- Misses        399      402       +3     
Flag Coverage Δ
project 91.01% <94.38%> (+0.06%) ⬆️
validator 90.36% <94.38%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
optimade/adapters/base.py 96.96% <83.33%> (-3.04%) ⬇️
optimade/models/utils.py 92.50% <91.66%> (-0.29%) ⬇️
optimade/adapters/structures/ase.py 98.63% <97.36%> (-1.37%) ⬇️
optimade/adapters/structures/adapter.py 100.00% <100.00%> (ø)
optimade/adapters/structures/pymatgen.py 98.50% <100.00%> (-0.16%) ⬇️
optimade/adapters/structures/utils.py 80.74% <100.00%> (+0.74%) ⬆️
optimade/models/structures.py 96.42% <100.00%> (+0.67%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ml-evs ml-evs changed the title Add ASE ingester and genearlize other ingestion utilities Add ASE ingester and generalize other ingestion utilities Feb 11, 2023
@ml-evs
Copy link
Member Author

ml-evs commented Feb 13, 2023

Hi @JPBergsma, will merge this tomorrow, let me know if you want to have a second look first and I'll hold off.

Copy link
Contributor

@JPBergsma JPBergsma left a comment

Choose a reason for hiding this comment

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

I had a quick look and everything seems OK.

@ml-evs ml-evs added this pull request to the merge queue Feb 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 14, 2023
@ml-evs ml-evs merged commit ad8a9fa into master Feb 14, 2023
@ml-evs ml-evs deleted the ml-evs/ase_adapter branch February 14, 2023 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters Issues pertaining to adapters (converters) enhancement New feature or request ergonomics Features that improve the usability of the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants