Skip to content

Commit

Permalink
Keep boto3 optional (#836)
Browse files Browse the repository at this point in the history
* Keep boto3 optional

* Remove boto3 from setup

* Linting

* Fix legacy molecules

* Remove url extraction test

* Do not skip charge density test

* Fix es test

* Remove print

* Linting

* Temp exclude new molecules resters from generic tests

* Linting
  • Loading branch information
munrojm committed Aug 15, 2023
1 parent 2352288 commit 5f15be1
Show file tree
Hide file tree
Showing 14 changed files with 48 additions and 161 deletions.
14 changes: 13 additions & 1 deletion mp_api/client/core/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from typing import Any, Dict, Generic, List, Optional, Tuple, TypeVar, Union
from urllib.parse import quote, urljoin

import boto3
import requests
from botocore import UNSIGNED
from botocore.config import Config
Expand All @@ -32,6 +31,11 @@
from mp_api.client.core.settings import MAPIClientSettings
from mp_api.client.core.utils import api_sanitize, validate_ids

try:
import boto3
except ImportError:
boto3 = None

try:
from pymatgen.core import __version__ as pmg_version # type: ignore
except ImportError: # pragma: no cover
Expand Down Expand Up @@ -132,6 +136,13 @@ def session(self) -> requests.Session:

@property
def s3_resource(self):
if boto3 is None:
raise MPRestError(
"boto3 not installed. To query charge density, "
"band structure, or density of states data first "
"install with: 'pip install boto3'"
)

if not self._s3_resource:
self._s3_resource = boto3.resource(
"s3", config=Config(signature_version=UNSIGNED)
Expand Down Expand Up @@ -329,6 +340,7 @@ def _query_open_data(self, bucket: str, prefix: str, key: str) -> dict:
Returns:
dict: MontyDecoded data
"""

ref = self.s3_resource.Object(bucket, f"{prefix}/{key}.json.gz") # type: ignore
bytes = ref.get()["Body"] # type: ignore

Expand Down
28 changes: 1 addition & 27 deletions mp_api/client/mprester.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from mp_api.client.core.settings import MAPIClientSettings
from mp_api.client.core.utils import validate_ids
from mp_api.client.routes import GeneralStoreRester, MessagesRester, UserSettingsRester
from mp_api.client.routes.legacy import LegacyMoleculesRester
from mp_api.client.routes.materials import (
AbsorptionRester,
AlloysRester,
Expand Down Expand Up @@ -57,19 +56,7 @@
ThermoRester,
XASRester,
)
from mp_api.client.routes.molecules import (
AssociatedMoleculeRester,
MoleculeRester,
MoleculesBondRester,
MoleculesOrbitalsRester,
MoleculesPartialChargesRester,
MoleculesPartialSpinsRester,
MoleculesRedoxRester,
MoleculesSummaryRester,
MoleculesTaskRester,
MoleculesThermoRester,
MoleculesVibrationRester,
)
from mp_api.client.routes.molecules import MoleculeRester

_DEPRECATION_WARNING = (
"MPRester is being modernized. Please use the new method suggested and "
Expand Down Expand Up @@ -124,20 +111,7 @@ class MPRester:
chemenv: ChemenvRester

# Molecules
molecules_bonding: MoleculesBondRester
molecules_associated: AssociatedMoleculeRester
molecules: MoleculeRester
molecules_orbital: MoleculesOrbitalsRester
molecules_partial_charges: MoleculesPartialChargesRester
molecules_partial_spins: MoleculesPartialSpinsRester
molecules_redox: MoleculesRedoxRester
molecules_summary: MoleculesSummaryRester
molecules_tasks: MoleculesTaskRester
molecules_thermo: MoleculesThermoRester
molecules_vibrations: MoleculesVibrationRester

# Legacy
legacy_jcesr: LegacyMoleculesRester

# Generic
doi: DOIRester
Expand Down
1 change: 0 additions & 1 deletion mp_api/client/routes/legacy/__init__.py

This file was deleted.

1 change: 0 additions & 1 deletion mp_api/client/routes/materials/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from .grain_boundary import GrainBoundaryRester
from .magnetism import MagnetismRester
from .materials import MaterialsRester
from .molecules import MoleculesRester
from .oxidation_states import OxidationStatesRester
from .phonon import PhononRester
from .piezo import PiezoRester
Expand Down
105 changes: 0 additions & 105 deletions mp_api/client/routes/materials/molecules.py

This file was deleted.

1 change: 1 addition & 0 deletions mp_api/client/routes/molecules/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from .bonds import MoleculesBondRester
from .jcesr import JcesrMoleculesRester
from .molecules import AssociatedMoleculeRester, MoleculeRester
from .orbitals import MoleculesOrbitalsRester
from .partial_charges import MoleculesPartialChargesRester
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
from mp_api.client.core import BaseRester


class LegacyMoleculesRester(BaseRester[MoleculesDoc]):
suffix = "legacy/jcesr"
class JcesrMoleculesRester(BaseRester[MoleculesDoc]):
suffix = "molecules/jcesr"
document_model = MoleculesDoc # type: ignore
primary_key = "task_id"

Expand Down
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
"requests>=2.23.0",
"monty>=2021.3.12",
"emmet-core>=0.54.0",
"boto3",
],
extras_require={
"all": ["emmet-core[all]>=0.54.0", "custodian", "mpcontribs-client", "boto3"],
Expand Down
Empty file removed tests/legacy/__init__.py
Empty file.
33 changes: 16 additions & 17 deletions tests/materials/test_charge_density.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import typing

import pytest
from emmet.core.charge_density import ChgcarDataDoc

from mp_api.client.routes.materials.charge_density import ChargeDensityRester

Expand Down Expand Up @@ -98,24 +97,24 @@ def test_download_for_task_ids(tmpdir, rester):
assert "mp-1791788.json.gz" in files


def test_extract_s3_url_info(rester):
url_doc_dict = {
"task_id": "mp-1896591",
"url": "https://minio.materialsproject.org/phuck/atomate_chgcar_fs/6021584c12afbe14911d1b8e",
"s3_url_prefix": "https://mp-volumetric.s3.amazonaws.com/atomate_chgcar_fs/",
"fs_id": "6021584c12afbe14911d1b8e",
"requested_datetime": {"$date": {"$numberLong": "1650389943209"}},
"expiry_datetime": None,
}
# def test_extract_s3_url_info(rester):
# url_doc_dict = {
# "task_id": "mp-1896591",
# "url": "https://minio.materialsproject.org/phuck/atomate_chgcar_fs/6021584c12afbe14911d1b8e",
# "s3_url_prefix": "https://mp-volumetric.s3.amazonaws.com/atomate_chgcar_fs/",
# "fs_id": "6021584c12afbe14911d1b8e",
# "requested_datetime": {"$date": {"$numberLong": "1650389943209"}},
# "expiry_datetime": None,
# }

url_doc = ChgcarDataDoc(**url_doc_dict)
# url_doc = ChgcarDataDoc(**url_doc_dict)

bucket, prefix = rester._extract_s3_url_info(url_doc)
# bucket, prefix = rester._extract_s3_url_info(url_doc)

assert bucket == "phuck"
assert prefix == "atomate_chgcar_fs"
# assert bucket == "phuck"
# assert prefix == "atomate_chgcar_fs"

bucket, prefix = rester._extract_s3_url_info(url_doc, use_minio=False)
# bucket, prefix = rester._extract_s3_url_info(url_doc, use_minio=False)

assert bucket == "mp-volumetric"
assert prefix == "atomate_chgcar_fs"
# assert bucket == "mp-volumetric"
# assert prefix == "atomate_chgcar_fs"
6 changes: 5 additions & 1 deletion tests/materials/test_electronic_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ def test_es_client(es_rester):
for entry in param_tuples:
param = entry[0]
if param not in es_excluded_params:
param_type = entry[1].__args__[0]
try:
param_type = entry[1].__args__[0]
except AttributeError:
param_type = entry[1]

q = None

if param in es_custom_field_tests:
Expand Down
4 changes: 2 additions & 2 deletions tests/legacy/test_jcesr.py → tests/molecules/test_jcesr.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
import pytest
from pymatgen.core.periodic_table import Element

from mp_api.client.routes.legacy.jcesr import LegacyMoleculesRester
from mp_api.client.routes.molecules.jcesr import JcesrMoleculesRester


@pytest.fixture
def rester():
rester = LegacyMoleculesRester()
rester = JcesrMoleculesRester()
yield rester
rester.session.close()

Expand Down
9 changes: 7 additions & 2 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,27 @@
"materials_xas",
"materials_elasticity",
"materials_fermi",
"molecules_vibrations",
# "alloys",
# "summary",
] # temp


mpr = MPRester()

# Temporarily ignore molecules resters while molecules query operators are changed
resters_to_test = [
rester for rester in mpr._all_resters if "molecule" not in rester.suffix
]


@pytest.mark.skipif(
os.environ.get("MP_API_KEY", None) is None, reason="No API key found."
)
@pytest.mark.parametrize("rester", mpr._all_resters)
@pytest.mark.parametrize("rester", resters_to_test)
def test_generic_get_methods(rester):
# -- Test generic search and get_data_by_id methods
name = rester.suffix.replace("/", "_")

if name not in ignore_generic:
if name not in key_only_resters:
doc = rester._query_resource_data(
Expand Down
2 changes: 1 addition & 1 deletion tests/test_mprester.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def test_get_phonon_data_by_material_id(self, mpr):
dos = mpr.get_phonon_dos_by_material_id("mp-2172")
assert isinstance(dos, PhononDos)

@pytest.mark.skip(reason="Test needs fixing with ENV variables")
# @pytest.mark.skip(reason="Test needs fixing with ENV variables")
def test_get_charge_density_data(self, mpr):
chgcar = mpr.get_charge_density_from_material_id("mp-149")
assert isinstance(chgcar, Chgcar)
Expand Down

0 comments on commit 5f15be1

Please sign in to comment.