From 038c5fe46ac9da0fe71485dbd56ea73842142249 Mon Sep 17 00:00:00 2001 From: Casper Welzel Andersen Date: Wed, 12 Oct 2022 19:11:04 +0200 Subject: [PATCH] Temp: Use pylint --- .github/mongo/load_data.py | 7 ++- .pre-commit-config.yaml | 23 ++++++- aiida_optimade/cli/cmd_calc.py | 13 ++-- aiida_optimade/cli/cmd_init.py | 18 +++--- aiida_optimade/cli/cmd_run.py | 10 ++- aiida_optimade/config.py | 9 ++- aiida_optimade/entry_collections.py | 6 +- aiida_optimade/main.py | 80 ++++++++++++------------ aiida_optimade/mappers/entries.py | 4 +- aiida_optimade/middleware.py | 1 + aiida_optimade/models/structures.py | 4 +- aiida_optimade/routers/info.py | 20 +++--- aiida_optimade/routers/utils.py | 11 ++-- aiida_optimade/transformers/aiida.py | 2 +- aiida_optimade/translators/cifs.py | 5 +- aiida_optimade/translators/entities.py | 10 ++- aiida_optimade/translators/structures.py | 13 ++-- pyproject.toml | 9 +-- tasks.py | 8 ++- 19 files changed, 146 insertions(+), 107 deletions(-) diff --git a/.github/mongo/load_data.py b/.github/mongo/load_data.py index 05818c73..cc6107ae 100755 --- a/.github/mongo/load_data.py +++ b/.github/mongo/load_data.py @@ -8,8 +8,11 @@ client = MongoClient("mongodb://localhost:27017") collection = client["aiida_optimade"]["structures"] -with open(Path(__file__).parent.joinpath("test_structures_mongo.json")) as handle: - data = bson.json_util.loads(handle.read()) +data = bson.json_util.loads( + Path(__file__) + .parent.joinpath("test_structures_mongo.json") + .read_text(encoding="utf8") +) try: print(f"Inserting {len(data)} structures into {collection.full_name}") diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b3ac507d..cf033083 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,7 +29,6 @@ repos: rev: 22.10.0 hooks: - id: black - name: Blacken - repo: local hooks: @@ -42,3 +41,25 @@ repos: .codecov.yml )$ language: system + - id: pylint + name: pylint + entry: pylint + args: + - --rcfile=pyproject.toml + - --extension-pkg-whitelist='pydantic' + language: python + types: [python] + require_serial: true + files: ^.*$ + exclude: ^tests/.*$ + - id: pylint-tests + name: pylint - tests + entry: pylint + args: + - --rcfile=pyproject.toml + - --extension-pkg-whitelist='pydantic' + - --disable=import-outside-toplevel,redefined-outer-name + language: python + types: [python] + require_serial: true + files: ^tests/.*$ diff --git a/aiida_optimade/cli/cmd_calc.py b/aiida_optimade/cli/cmd_calc.py index 345b3432..7be3f22b 100644 --- a/aiida_optimade/cli/cmd_calc.py +++ b/aiida_optimade/cli/cmd_calc.py @@ -1,7 +1,10 @@ # pylint: disable=protected-access,too-many-locals,too-many-branches +import traceback from typing import Tuple import click +from aiida import load_profile +from aiida.cmdline.utils import echo from tqdm import tqdm from aiida_optimade.cli.cmd_aiida_optimade import cli @@ -35,11 +38,10 @@ help="Suppress informational output.", ) @click.pass_obj -def calc(obj: dict, fields: Tuple[str], force_yes: bool, silent: bool): +def calc( + obj: dict, fields: Tuple[str], force_yes: bool, silent: bool +): # pylint: disable=too-many-statements """Calculate OPTIMADE fields in the AiiDA database.""" - from aiida import load_profile - from aiida.cmdline.utils import echo - # The default aiida.cmdline loglevel inherit from aiida loglevel is REPORT # Here we use INFO loglevel for the operations echo.CMDLINE_LOGGER.setLevel("INFO") @@ -52,6 +54,7 @@ def calc(obj: dict, fields: Tuple[str], force_yes: bool, silent: bool): try: with disable_logging(): + # pylint: disable=import-outside-toplevel from aiida_optimade.routers.structures import STRUCTURES extras_key = STRUCTURES.resource_mapper.PROJECT_PREFIX.split(".")[1] @@ -139,8 +142,6 @@ def calc(obj: dict, fields: Tuple[str], force_yes: bool, silent: bool): echo.echo_warning("Aborted!") return except Exception as exc: # pylint: disable=broad-except - import traceback - exception = traceback.format_exc() LOGGER.error("Full exception from 'aiida-optimade calc' CLI:\n%s", exception) diff --git a/aiida_optimade/cli/cmd_init.py b/aiida_optimade/cli/cmd_init.py index 553326df..ce3ee4e8 100644 --- a/aiida_optimade/cli/cmd_init.py +++ b/aiida_optimade/cli/cmd_init.py @@ -1,8 +1,12 @@ # pylint: disable=protected-access,too-many-statements +import traceback from pathlib import Path from typing import IO, Generator, Iterator, List, Union +import bson.json_util import click +from aiida import load_profile +from aiida.cmdline.utils import echo from tqdm import tqdm from aiida_optimade.cli.cmd_aiida_optimade import cli @@ -43,11 +47,10 @@ help="Filename to load as database (currently only usable for MongoDB).", ) @click.pass_obj -def init(obj: dict, force: bool, silent: bool, mongo: bool, filename: str): +def init( + obj: dict, force: bool, silent: bool, mongo: bool, filename: str +): # pylint: disable=too-many-locals,too-many-branches """Initialize an AiiDA database to be served with AiiDA-OPTIMADE.""" - from aiida import load_profile - from aiida.cmdline.utils import echo - # The default aiida.cmdline loglevel inherit from aiida loglevel is REPORT # Here we use INFO loglevel for the operations echo.CMDLINE_LOGGER.setLevel("INFO") @@ -65,6 +68,7 @@ def init(obj: dict, force: bool, silent: bool, mongo: bool, filename: str): try: with disable_logging(): + # pylint: disable=import-outside-toplevel from aiida_optimade.routers.structures import STRUCTURES if mongo: @@ -136,8 +140,6 @@ def init(obj: dict, force: bool, silent: bool, mongo: bool, filename: str): "Passing a filename currently only works for a MongoDB backend" ) - import bson.json_util - updated_pks = range(len(STRUCTURES_MONGO)) chunk_size = 2**24 # 16 MB @@ -148,7 +150,7 @@ def init(obj: dict, force: bool, silent: bool, mongo: bool, filename: str): "consider using --force to first drop the collection, if possible." ) - with open(filename, "r") as handle: + with open(filename, "r", encoding="utf8") as handle: if silent: all_chunks = read_chunks(handle, chunk_size=chunk_size) else: @@ -196,8 +198,6 @@ def init(obj: dict, force: bool, silent: bool, mongo: bool, filename: str): entries=entries if mongo else None, ) except Exception as exc: # pylint: disable=broad-except - import traceback - exception = traceback.format_exc() LOGGER.error("Full exception from 'aiida-optimade init' CLI:\n%s", exception) diff --git a/aiida_optimade/cli/cmd_run.py b/aiida_optimade/cli/cmd_run.py index 749a050c..52bd9c0c 100644 --- a/aiida_optimade/cli/cmd_run.py +++ b/aiida_optimade/cli/cmd_run.py @@ -1,5 +1,9 @@ # pylint: disable=too-many-arguments +import os + import click +import uvicorn +from aiida import load_profile from aiida_optimade.cli.cmd_aiida_optimade import cli from aiida_optimade.cli.options import LOGGING_LEVELS @@ -45,10 +49,6 @@ @click.pass_obj def run(obj: dict, log_level: str, debug: bool, host: str, port: int, reload: bool): """Run AiiDA-OPTIMADE server.""" - import os - - import uvicorn - if obj.get("dev", False): debug = True @@ -67,8 +67,6 @@ def run(obj: dict, log_level: str, debug: bool, host: str, port: int, reload: bo os.environ["AIIDA_OPTIMADE_LOG_LEVEL"] = log_level.upper() if os.getenv("AIIDA_PROFILE") is None: - from aiida import load_profile - try: profile: str = obj.get("profile").name except AttributeError: diff --git a/aiida_optimade/config.py b/aiida_optimade/config.py index 09ed38f6..f62dcae1 100644 --- a/aiida_optimade/config.py +++ b/aiida_optimade/config.py @@ -5,10 +5,17 @@ class CustomServerConfig(ServerConfig): + """Custom AiiDA-OPTIMADE server config. + + This extends the server config class from the OPTIMADE Python server. + """ query_group: Optional[str] = Field( None, - description="The AiiDA Group containing the data that will be served, allowing one to serve a curated set of data from a given database.", + description=( + "The AiiDA Group containing the data that will be served, allowing one to " + "serve a curated set of data from a given database." + ), ) diff --git a/aiida_optimade/entry_collections.py b/aiida_optimade/entry_collections.py index 8a43098b..29e9e79d 100644 --- a/aiida_optimade/entry_collections.py +++ b/aiida_optimade/entry_collections.py @@ -1,4 +1,5 @@ import warnings +from copy import deepcopy from typing import Any, Dict, List, Optional, Set, Tuple, Union from aiida.orm import Group @@ -96,7 +97,7 @@ def set_data_returned(self, **criteria): and criteria.get("filters", {}) != self._latest_filter ): for key in ["limit", "offset"]: - if key in list(criteria.keys()): + if key in list(criteria): del criteria[key] self._latest_filter = criteria.get("filters", {}).copy() LOGGER.debug("Setting data_returned using filter: %s", self._latest_filter) @@ -130,7 +131,7 @@ def count(self, **kwargs) -> int: "offset": kwargs.get("offset", None), } else: - for limiting_param in {"filters", "limit", "offset"}: + for limiting_param in ["filters", "limit", "offset"]: if kwargs.get(limiting_param, None) != self._count.get( limiting_param, None ): @@ -438,7 +439,6 @@ def _find_extras_fields(self, filters: Union[dict, list]) -> None: parameter. """ - from copy import deepcopy def __filter_fields_util( # pylint: disable=unused-private-member _filters: Union[dict, list] diff --git a/aiida_optimade/main.py b/aiida_optimade/main.py index 4855fd74..d6882f88 100644 --- a/aiida_optimade/main.py +++ b/aiida_optimade/main.py @@ -1,3 +1,4 @@ +# pylint: disable=ungrouped-imports import json import os import warnings @@ -117,46 +118,47 @@ async def startup(): LOGGER.info("AiiDA Profile: %s", profile_name) # Load links - with open(Path(__file__).parent.joinpath("data/links.json").resolve()) as handle: - data = json.load(handle) - - if CONFIG.debug: - data.append( - { - "id": "local", - "type": "links", - "name": "Local server", - "description": ( - "Locally running instance of the AiiDA-OPTIMADE server using " - f"AiiDA profile {profile_name!r}." - ), - "base_url": "http://localhost:5000", - "homepage": "https://github.com/aiidateam/aiida-optimade", - "link_type": "child", - } - ) + data = json.loads( + Path(__file__).parent.joinpath("data/links.json").resolve().read_bytes() + ) - processed = [] - for link in data: - link["_id"] = {"$oid": mongo_id_for_database(link["id"], link["type"])} - processed.append(link) - - LOGGER.info("Loading links") - if CONFIG.database_backend == SupportedBackend.MONGODB: - LOGGER.info(" Using real MongoDB.") - if links.LINKS.count( - filter={"id": {"$in": [_["id"] for _ in processed]}} - ) != len(links.LINKS): - LOGGER.info( - " Will drop and reinsert links data in %s", - links.LINKS.collection.full_name, - ) - links.LINKS.collection.drop() - links.LINKS.collection.insert_many( - bson.json_util.loads(bson.json_util.dumps(processed)), - ) - else: - LOGGER.info(" Using mock MongoDB.") + if CONFIG.debug: + data.append( + { + "id": "local", + "type": "links", + "name": "Local server", + "description": ( + "Locally running instance of the AiiDA-OPTIMADE server using " + f"AiiDA profile {profile_name!r}." + ), + "base_url": "http://localhost:5000", + "homepage": "https://github.com/aiidateam/aiida-optimade", + "link_type": "child", + } + ) + + processed = [] + for link in data: + link["_id"] = {"$oid": mongo_id_for_database(link["id"], link["type"])} + processed.append(link) + + LOGGER.info("Loading links") + if CONFIG.database_backend == SupportedBackend.MONGODB: + LOGGER.info(" Using real MongoDB.") + if links.LINKS.count( + filter={"id": {"$in": [_["id"] for _ in processed]}} + ) != len(links.LINKS): + LOGGER.info( + " Will drop and reinsert links data in %s", + links.LINKS.collection.full_name, + ) + links.LINKS.collection.drop() links.LINKS.collection.insert_many( bson.json_util.loads(bson.json_util.dumps(processed)), ) + else: + LOGGER.info(" Using mock MongoDB.") + links.LINKS.collection.insert_many( + bson.json_util.loads(bson.json_util.dumps(processed)), + ) diff --git a/aiida_optimade/mappers/entries.py b/aiida_optimade/mappers/entries.py index 69feca1c..fccaf9b7 100644 --- a/aiida_optimade/mappers/entries.py +++ b/aiida_optimade/mappers/entries.py @@ -37,7 +37,9 @@ def all_aliases(cls) -> Tuple[Tuple[str, str]]: ) @classmethod - def map_back(cls, entity_properties: Dict[str, Any]) -> dict: + def map_back( # pylint: disable=arguments-renamed + cls, entity_properties: Dict[str, Any] + ) -> dict: """Map properties from AiiDA to OPTIMADE Parameters: diff --git a/aiida_optimade/middleware.py b/aiida_optimade/middleware.py index 8e521ee0..6d5d546c 100644 --- a/aiida_optimade/middleware.py +++ b/aiida_optimade/middleware.py @@ -1,3 +1,4 @@ +# pylint: disable=too-few-public-methods import urllib.parse from optimade.server.routers.utils import BASE_URL_PREFIXES diff --git a/aiida_optimade/models/structures.py b/aiida_optimade/models/structures.py index c89c7984..e2368873 100644 --- a/aiida_optimade/models/structures.py +++ b/aiida_optimade/models/structures.py @@ -7,11 +7,11 @@ ) from pydantic import Field +from aiida_optimade.config import CONFIG + def prefix_provider(string: str) -> str: """Prefix string with `_{provider}_`""" - from optimade.server.config import CONFIG - if string in CONFIG.provider_fields.get("structures", []): return f"_{CONFIG.provider.prefix}_{string}" return string diff --git a/aiida_optimade/routers/info.py b/aiida_optimade/routers/info.py index f9346633..e281e434 100644 --- a/aiida_optimade/routers/info.py +++ b/aiida_optimade/routers/info.py @@ -1,12 +1,19 @@ # pylint: disable=missing-function-docstring -import urllib +import urllib.parse from typing import Union from fastapi import APIRouter, HTTPException, Request from optimade import __api_version__ -from optimade.models import EntryInfoResponse, ErrorResponse, InfoResponse +from optimade.models import ( + BaseInfoAttributes, + BaseInfoResource, + EntryInfoResource, + EntryInfoResponse, + ErrorResponse, + InfoResponse, +) from optimade.server.config import CONFIG -from optimade.server.routers.utils import meta_values +from optimade.server.routers.utils import get_base_url, meta_values from aiida_optimade.models import StructureResource from aiida_optimade.utils import retrieve_queryable_properties @@ -24,9 +31,6 @@ tags=["Info"], ) def get_info(request: Request): - from optimade.models import BaseInfoAttributes, BaseInfoResource - from optimade.server.routers.utils import get_base_url - parse_result = urllib.parse.urlparse(str(request.url)) base_url = get_base_url(parse_result) @@ -41,7 +45,7 @@ def get_info(request: Request): api_version=__api_version__, available_api_versions=[ { - "url": f"{base_url}/v{__api_version__.split('-')[0].split('+')[0].split('.')[0]}", + "url": f"{base_url}/v{__api_version__.split('-', maxsplit=1)[0].split('+', maxsplit=1)[0].split('.', maxsplit=1)[0]}", # pylint: disable=line-too-long "version": __api_version__, } ], @@ -69,8 +73,6 @@ def get_info(request: Request): tags=["Info"], ) def get_info_entry(request: Request, entry: str): - from optimade.models import EntryInfoResource - valid_entry_info_endpoints = ENTRY_INFO_SCHEMAS.keys() if entry not in valid_entry_info_endpoints: raise HTTPException( diff --git a/aiida_optimade/routers/utils.py b/aiida_optimade/routers/utils.py index 1553f43b..23717871 100644 --- a/aiida_optimade/routers/utils.py +++ b/aiida_optimade/routers/utils.py @@ -2,12 +2,17 @@ import urllib.parse from typing import Union +from aiida.manage.manager import get_manager from fastapi import HTTPException, Request from optimade.models import EntryResponseMany, EntryResponseOne, ToplevelLinks from optimade.server.config import CONFIG from optimade.server.entry_collections.mongo import MongoCollection from optimade.server.query_params import EntryListingQueryParams, SingleEntryQueryParams -from optimade.server.routers.utils import handle_response_fields, meta_values +from optimade.server.routers.utils import ( + get_base_url, + handle_response_fields, + meta_values, +) from aiida_optimade.entry_collections import AiidaCollection @@ -16,8 +21,6 @@ def handle_pagination( request: Request, more_data_available: bool, nresults: int ) -> dict: """Handle pagination for request with number of results equal nresults""" - from optimade.server.routers.utils import get_base_url - pagination = {} # "prev" @@ -151,8 +154,6 @@ def wrapper(*args, **kwargs): try: value = func(*args, **kwargs) finally: - from aiida.manage.manager import get_manager - get_manager().get_backend().get_session().close() return value diff --git a/aiida_optimade/transformers/aiida.py b/aiida_optimade/transformers/aiida.py index 32899a5a..13e0b99f 100644 --- a/aiida_optimade/transformers/aiida.py +++ b/aiida_optimade/transformers/aiida.py @@ -1,4 +1,4 @@ -# pylint: disable=no-self-use,too-many-public-methods +# pylint: disable=too-many-public-methods from lark import v_args from optimade.filtertransformers import BaseTransformer, Quantity from optimade.server.exceptions import BadRequest diff --git a/aiida_optimade/translators/cifs.py b/aiida_optimade/translators/cifs.py index 85df6811..d270a222 100644 --- a/aiida_optimade/translators/cifs.py +++ b/aiida_optimade/translators/cifs.py @@ -2,6 +2,8 @@ from aiida.orm.nodes.data.cif import CifData from aiida.orm.nodes.data.structure import StructureData +from aiida.tools.data.cif import InvalidOccupationsError +from pymatgen.io.cif import CifParser from aiida_optimade.translators.structures import StructureDataTranslator @@ -22,9 +24,6 @@ def _get_aiida_structure_pymatgen_inline(cif, **kwargs) -> StructureData: .. note:: requires pymatgen module. """ - from aiida.tools.data.cif import InvalidOccupationsError - from pymatgen.io.cif import CifParser - parameters = kwargs.get("parameters", {}) constructor_kwargs = {} diff --git a/aiida_optimade/translators/entities.py b/aiida_optimade/translators/entities.py index 0a90d21c..aaffddee 100644 --- a/aiida_optimade/translators/entities.py +++ b/aiida_optimade/translators/entities.py @@ -1,10 +1,13 @@ from typing import Any, List, Union +import bson.json_util from aiida import orm from aiida.orm.nodes import Node from aiida.orm.querybuilder import QueryBuilder from aiida_optimade.common import LOGGER, AiidaEntityNotFound +from aiida_optimade.routers.structures import STRUCTURES_MONGO +from aiida_optimade.translators.utils import hex_to_floats __all__ = ("AiidaEntityTranslator",) @@ -33,7 +36,7 @@ def _get_unique_node_property( raise AiidaEntityNotFound( f"Could not find {self.AIIDA_ENTITY} with PK {self._pk}." ) - res = query.first() + res: list = query.first() del query return res if len(res) > 1 else res[0] @@ -80,11 +83,6 @@ def store_attributes(self, mongo: bool = False) -> None: def _store_attributes_mongo(self) -> None: """Store new attributes in MongoDB collection""" - import bson.json_util - - from aiida_optimade.routers.structures import STRUCTURES_MONGO - from aiida_optimade.translators.utils import hex_to_floats - optimade = STRUCTURES_MONGO.collection.find_one(filter={"id": self._pk}) if optimade: optimade.update(self.new_attributes) diff --git a/aiida_optimade/translators/structures.py b/aiida_optimade/translators/structures.py index 95a792e4..34e23ead 100644 --- a/aiida_optimade/translators/structures.py +++ b/aiida_optimade/translators/structures.py @@ -1,9 +1,15 @@ # pylint: disable=line-too-long,too-many-public-methods import itertools +import re from math import fsum from typing import Any, List, Union -from aiida.orm.nodes.data.structure import StructureData +from aiida.orm.nodes.data.structure import ( + _SUM_THRESHOLD, + StructureData, + get_formula, + get_symbols_string, +) from optimade.models.utils import ANONYMOUS_ELEMENTS from aiida_optimade.common import AiidaError, OptimadeIntegrityError @@ -82,7 +88,6 @@ def get_symbols_set(self): def has_vacancies(self): """Copy of aiida.orm.StructureData:has_vacancies""" - from aiida.orm.nodes.data.structure import _SUM_THRESHOLD def kind_has_vacancies(weights): """Copy of aiida.orm.Kinds:has_vacancies""" @@ -93,8 +98,6 @@ def kind_has_vacancies(weights): def get_formula(self, mode="hill", separator=""): """Copy of aiida.orm.StructureData:get_formula()""" - from aiida.orm.nodes.data.structure import get_formula, get_symbols_string - kind = None symbol_list = [] for site in self._sites: @@ -380,8 +383,6 @@ def species(self) -> List[dict]: Species can be pure chemical elements, or virtual-crystal atoms representing a statistical occupation of a given site by multiple chemical elements. """ - import re - attribute = "species" if attribute in self.new_attributes: diff --git a/pyproject.toml b/pyproject.toml index d518e967..5ceec59e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -93,13 +93,14 @@ allow_redefinition = true [tool.pylint.messages_control] max-line-length = 88 disable = [ - "import-outside-toplevel", + "duplicate-code", "missing-module-docstring", - "locally-disabled", - "bad-continuation", + # "locally-disabled", "fixme", - "too-many-instance-attributes", + # "too-many-instance-attributes", ] +good-names = ['i', 'j', 'k', 'ex', 'Run', '_', 'pk'] +max-attributes = 10 [tool.pytest.ini_options] minversion = "7.0" diff --git a/tasks.py b/tasks.py index 48c07210..9371f97d 100644 --- a/tasks.py +++ b/tasks.py @@ -1,3 +1,4 @@ +# pylint: disable=import-outside-toplevel import re from typing import Tuple @@ -6,12 +7,12 @@ def update_file(filename: str, sub_line: Tuple[str, str], strip: str = None): """Utility function for tasks to read, update, and write files""" - with open(filename, "r") as handle: + with open(filename, "r", encoding="utf8") as handle: lines = [ re.sub(sub_line[0], sub_line[1], line.rstrip(strip)) for line in handle ] - with open(filename, "w") as handle: + with open(filename, "w", encoding="utf8") as handle: handle.write("\n".join(lines)) handle.write("\n") @@ -30,7 +31,8 @@ def optimade_req(_, ver=""): optimade_init = requests.get( "https://raw.githubusercontent.com/Materials-Consortia/optimade-python-tools" - f"/v{ver}/optimade/__init__.py" + f"/v{ver}/optimade/__init__.py", + timeout=30, ) if optimade_init.status_code != 200: raise RuntimeError(f"{ver} does not seem to be published on GitHub")