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

Major update to Linting #162

Merged
merged 8 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 11 additions & 52 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,56 +1,15 @@
exclude: ^(docs|tests)

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
- id: check-yaml
- id: fix-encoding-pragma
args:
- --remove
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/PyCQA/autoflake
rev: v2.2.1
hooks:
- id: autoflake
- repo: https://github.com/psf/black
rev: 23.9.1
hooks:
- id: black
- repo: https://github.com/asottile/blacken-docs
rev: 1.16.0
hooks:
- id: blacken-docs
additional_dependencies: [black]
exclude: README.md
- repo: https://github.com/pycqa/isort
rev: 5.12.0
hooks:
- id: isort
- repo: https://github.com/pycqa/flake8
rev: 6.1.0
hooks:
- id: flake8
entry: pflake8
files: ^src/
additional_dependencies:
- pyproject-flake8==6.1.0
- flake8-bugbear==22.12.6
- flake8-typing-imports==1.14.0
- flake8-docstrings==1.6.0
- flake8-rst-docstrings==0.3.0
- flake8-rst==0.8.0
- repo: https://github.com/pycqa/pydocstyle
rev: 6.3.0
hooks:
- id: pydocstyle
additional_dependencies: ["tomli==2.0.1"]
- repo: https://github.com/pre-commit/pygrep-hooks
rev: v1.10.0
hooks:
- id: python-use-type-annotations
- id: rst-backticks
- id: rst-directive-colons
- id: rst-inline-touching-normal
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.1.8
hooks:
# Run the linter.
- id: ruff
args: [ --fix ]
# Run the formatter.
- id: ruff-format
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.5.1
hooks:
Expand Down
10 changes: 5 additions & 5 deletions docs/source/content/formation-energy.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@
"\n",
"$$\n",
"\\begin{aligned}\n",
"\\mu_{\\rm Ga} + \\mu_{\\rm N} &= \\Delta H_{\\rm GaN} \\\\\n",
"5\\mu_{\\rm Mg} + 2\\mu_{\\rm Ga} + \\mu_{\\rm N} &\\leq \\Delta H_{\\rm Mg_5Ga_2} \\\\\n",
"3\\mu_{\\rm Mg} + 2\\mu_{\\rm N} &\\leq \\Delta H_{\\rm Mg_3N_2} \\\\\n",
"\\Delta\\mu_{\\rm Ga} + \\Delta\\mu_{\\rm N} &= \\Delta H_{\\rm GaN} \\\\\n",
"5\\Delta\\mu_{\\rm Mg} + 2\\Delta\\mu_{\\rm Ga} + \\Delta\\mu_{\\rm N} &\\leq \\Delta H_{\\rm Mg_5Ga_2} \\\\\n",
"3\\Delta\\mu_{\\rm Mg} + 2\\Delta\\mu_{\\rm N} &\\leq \\Delta H_{\\rm Mg_3N_2} \\\\\n",
"\\end{aligned}\n",
"$$\n",
"\n",
Expand All @@ -130,9 +130,9 @@
"outputs": [],
"source": [
"for i, p in enumerate(fed.chempot_limits):\n",
" print(f\"Chemical potential limits for point {i}\")\n",
" print(f\"Limits for the chemical potential changes for point {i}\")\n",
" for k,v in p.items():\n",
" print(f\"μ_{k} = {v:.2f} eV\")"
" print(f\"Δμ_{k} = {v:.2f} eV\")"
]
},
{
Expand Down
6 changes: 5 additions & 1 deletion docs/source/content/freysoldt-correction.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,11 @@
" )\n",
" plot_plnr_avg(\n",
" freysoldt_results[q].metadata[\"plot_data\"][direction], ax=axs[direction, 0]\n",
" )"
" )\n",
" axs[direction, 0].set_title(f\"q={q} direction={direction} (Python)\")\n",
" axs[direction, 1].set_title(f\"q={q} direction={direction} (SXDefectAlign)\")\n",
" axs[direction, 0].set_xlabel(\"\")\n",
" axs[direction, 1].set_xlabel(\"\")"
]
},
{
Expand Down
4 changes: 3 additions & 1 deletion docs/source/content/nonradiative.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@
"\n",
"The potential energy surface of the defect in a particular charge state is given by:\n",
"\n",
"$$\\frac{1}{2} \\omega^2 Q^2$$\n"
"$$\\frac{1}{2} \\omega^2 Q^2$$\n",
"\n",
"where $\\omega$ is the harmonic frequency and $Q$ is the mass-weighted displacement (configuration coordinate) of the defect as it changes between different charge states."
]
},
{
Expand Down
29 changes: 19 additions & 10 deletions pymatgen/analysis/defects/ccd.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,29 @@
from __future__ import annotations

import logging
from ctypes import Structure
from dataclasses import dataclass
from itertools import groupby
from pathlib import Path
from typing import Optional, Sequence, Tuple
from typing import TYPE_CHECKING, Optional, Sequence, Tuple

import numpy as np
import numpy.typing as npt
from matplotlib.axes import Axes
from monty.json import MSONable
from pymatgen.electronic_structure.core import Spin
from pymatgen.io.vasp.optics import DielectricFunctionCalculator
from pymatgen.io.vasp.outputs import WSWQ, BandStructure, Procar, Vasprun, Waveder
from pymatgen.io.vasp.outputs import WSWQ, Procar, Vasprun
from scipy.optimize import curve_fit

from .constants import AMU2KG, ANGS2M, EV2J, HBAR_EV, KB
from .recombination import get_SRH_coef
from .utils import get_localized_states, get_zfile, sort_positive_definite

if TYPE_CHECKING:
from ctypes import Structure
from pathlib import Path

import numpy.typing as npt
from matplotlib.axes import Axes
from pymatgen.io.vasp.outputs import BandStructure, Waveder

# Copyright (c) Pymatgen Development Team.
# Distributed under the terms of the MIT License.

Expand Down Expand Up @@ -142,13 +146,16 @@ def from_vaspruns(
Args:
vaspruns: A list of Vasprun objects.
charge_state: The charge state for the defect.
spin_index: The index of the spin channel to use. If None, the spin channel
is determined by the channel with the most localized state.
relaxed_index: The index of the relaxed structure in the list of structures.
defect_band: The the index of the defect band since the defect for different
kpoints and spins presented as `[(band, kpt, spin), ...]`.
procar: A Procar object. Used to identify the defect band if the defect_band_index is not provided.
store_bandstructure: Whether to store the bandstructure of the relaxed defect calculation.
Defaults to False to save space.
get_band_structure_kwargs: Keyword arguments to pass to the ``get_band_structure`` method.
band_window: The number of bands above and below the defect band to include when searching for the defect band.
**kwargs: Additional keyword arguments to pass to the constructor.

Returns:
Expand Down Expand Up @@ -213,13 +220,13 @@ def _parse_vasprun(vasprun: Vasprun):
spin: list(bands)
for spin, bands in groupby(defect_band_2s, lambda x: x[2])
}
avg_localization = {
spin: np.average([val for _, _, _, val in bands])
min_localization = {
spin: np.min([val for _, _, _, val in bands])
for spin, bands in defect_band_grouped.items()
}
if spin_index is None:
# get the most localized spin
spin_index = min(avg_localization, key=avg_localization.get)
spin_index = min(min_localization, key=min_localization.get)
# drop the val
defect_band = [r_[:3] for r_ in defect_band_grouped[spin_index]]

Expand Down Expand Up @@ -761,7 +768,7 @@ def _get_ks_ediff(
for ispin, eigs in bandstructure.bands.items():
spin_index = 0 if ispin == Spin.up else 1
res[ispin] = np.zeros_like(eigs)
for ikpt, kpt in enumerate(bandstructure.kpoints):
for ikpt, _kpt in enumerate(bandstructure.kpoints):
iband = b_at_kpt_and_spin.get((ikpt, spin_index), None)
if iband is None:
continue
Expand All @@ -780,6 +787,8 @@ def plot_pes(
hd: HarmonicDefect object
x_shift: shift the PES by this amount in the x-direction
y_shift: shift the PES by this amount in the y-direction
width: the x-range for the PES plot
ax: matplotlib axes object

Returns:
None
Expand Down
40 changes: 24 additions & 16 deletions pymatgen/analysis/defects/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,22 @@
import logging
from abc import ABCMeta, abstractmethod, abstractproperty
from enum import Enum
from typing import Dict
from typing import TYPE_CHECKING, Dict

import numpy as np
from monty.json import MSONable
from pymatgen.analysis.defects.supercells import get_sc_fromstruct
from pymatgen.analysis.structure_matcher import ElementComparator, StructureMatcher
from pymatgen.core import Element, PeriodicSite, Species, Structure
from pymatgen.core import Element, PeriodicSite, Species
from pymatgen.core.periodic_table import DummySpecies
from pymatgen.symmetry.analyzer import SpacegroupAnalyzer
from pymatgen.symmetry.structure import SymmetrizedStructure

from pymatgen.analysis.defects.supercells import get_sc_fromstruct

from .utils import get_plane_spacing

if TYPE_CHECKING:
from pymatgen.core import Structure
from pymatgen.symmetry.structure import SymmetrizedStructure

# TODO Possible redesign idea: ``DefectSite`` class defined with a defect object.
# This makes some of the accounting logic a bit harder since we will probably
# just have one concrete ``Defect`` class so you can write custom multiplicity functions
Expand Down Expand Up @@ -88,7 +90,7 @@
# check oxi_states assigned and not all zero
if all(specie.oxi_state == 0 for specie in self.structure.species):
self.structure.add_oxidation_state_by_guess()
except: # pragma: no cover
except Exception:

Check warning on line 93 in pymatgen/analysis/defects/core.py

View check run for this annotation

Codecov / codecov/patch

pymatgen/analysis/defects/core.py#L93

Added line #L93 was not covered by tests
self.structure.add_oxidation_state_by_guess()
self.oxi_state = self._guess_oxi_state()
else:
Expand Down Expand Up @@ -363,7 +365,8 @@
"""

def __init__(self, name: str, bulk_formula: str, element_changes: dict) -> None:
"""
"""Initialize a NamedDefect object.

Args:
name: The name of the defect.
bulk_formula: The formula of the bulk structure.
Expand Down Expand Up @@ -410,6 +413,7 @@
return self.__repr__() == __value.__repr__()

def __repr__(self) -> str:
"""String representation of the NamedDefect."""
return f'{self.bulk_formula}:{"+".join(sorted(self.name.split("+")))}'


Expand Down Expand Up @@ -505,7 +509,8 @@
site: Replace the nearest site with this one.
multiplicity: The multiplicity of the defect.
oxi_state: The oxidation state of the defect, if not specified,
this will be determined automatically.
this will be determined automatically.
**kwargs: Additional kwargs to pass to the Defect constructor.
"""
super().__init__(structure, site, multiplicity, oxi_state, **kwargs)

Expand Down Expand Up @@ -630,6 +635,8 @@
multiplicity: The multiplicity of the defect.
oxi_state: The oxidation state of the defect, if not specified,
this will be determined automatically.
equivalent_sites: A list of equivalent sites for the defect in the structure.
**kwargs: Additional kwargs to pass to the Defect constructor.
"""
super().__init__(
structure, site, multiplicity, oxi_state, equivalent_sites, **kwargs
Expand Down Expand Up @@ -792,6 +799,7 @@

@property
def latex_name(self) -> str:
"""Get the latex name of the defect."""
single_names = [d.latex_name for d in self.defects]
return "$+$".join(single_names)

Expand Down Expand Up @@ -911,9 +919,10 @@
min_distance: float | None = None,
site_indices: list | None = None,
) -> None:
"""
Performs a random perturbation of the sites in a structure to break
symmetries.
"""Performs a random perturbation.

Perturb the sites in a structure to break symmetry. This is useful for
finding energy minimum configurations.

Args:
structure (Structure): Input structure.
Expand Down Expand Up @@ -1015,11 +1024,10 @@
def _get_defect_name(element_diff: dict) -> str:
"""Get the name of the defect.

Parse the defect structure and bulk structure to get the name of the defect.
Parse the change in different elements to get the name of the defect.

Args:
defect_sc: The defect structure.
bulk_sc: The bulk structure.
element_diff: A dictionary representing the species changes of the defect.

Returns:
str: The name of the defect, if the defect is a complex, the names of the
Expand Down Expand Up @@ -1047,12 +1055,12 @@

# get the different vacancy names
vac_names = []
for el, cnt in removed_list:
for el, _cnt in removed_list:
vac_names.append(f"v_{el}")

# get the different interstitial names
int_names = []
for el, cnt in added_list:
for el, _cnt in added_list:
int_names.append(f"{el}_i")

# combine the names
Expand Down
Loading
Loading