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

Ruff #250

Merged
merged 7 commits into from
Feb 24, 2023
Merged

Ruff #250

Show file tree
Hide file tree
Changes from 5 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
22 changes: 9 additions & 13 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ Include a summary of major changes in bullet points:
* List all new dependencies needed and justify why. While adding dependencies that bring
significantly useful functionality is perfectly fine, adding ones that add trivial
functionality, e.g., to use one single easily implementable function, is frowned upon.
Provide a justification why that dependency is needed. Especially frowned upon are
circular dependencies.
Justify why that dependency is needed. Especially frowned upon are circular dependencies.

## TODO (if any)

Expand All @@ -26,20 +25,17 @@ title.

Before a pull request can be merged, the following items must be checked:

- [ ] Code is in the [standard Python style](https://www.python.org/dev/peps/pep-0008/).
* [ ] Code is in the [standard Python style](https://www.python.org/dev/peps/pep-0008/).
The easiest way to handle this is to run the following in the **correct sequence** on
your local machine. Start with running [black](
https://black.readthedocs.io/en/stable/index.html) on your new code. This will
your local machine. Start with running [black](https://black.readthedocs.io/en/stable/index.html) on your new code. This will
automatically reformat your code to PEP8 conventions and removes most issues. Then run
[pycodestyle](https://pycodestyle.readthedocs.io/en/latest/), followed by [flake8](
http://flake8.pycqa.org/en/latest/).
- [ ] Docstrings have been added in the[Numpy docstring format](
https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_numpy.html).
Run [pydocstyle](http://www.pydocstyle.org/en/2.1.1/index.html) on your code.
- [ ] Type annotations are **highly** encouraged. Run [mypy](http://mypy-lang.org/) to
[ruff](https://ruff.rs).
* [ ] Docstrings have been added in the [Numpy docstring format](https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_numpy.html).
Run [ruff](https://beta.ruff.rs/docs/rules/#pydocstyle-d) on your code.
* [ ] Type annotations are **highly** encouraged. Run [mypy](http://mypy-lang.org) to
type check your code.
- [ ] Tests have been added for any new functionality or bug fixes.
- [ ] All linting and tests pass.
* [ ] Tests have been added for any new functionality or bug fixes.
* [ ] All linting and tests pass.

Note that the CI system will run all the above checks. But it will be much more
efficient if you already fix most errors prior to submitting the PR. It is highly
Expand Down
18 changes: 5 additions & 13 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ default_language_version:
python: python3
exclude: '^src/atomate2/vasp/schemas/calc_types/'
repos:
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.0.250
hooks:
- id: ruff
args: [--fix]
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
Expand All @@ -10,10 +15,6 @@ repos:
args: [--remove]
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/myint/autoflake
rev: v2.0.0
hooks:
- id: autoflake
- repo: https://github.com/psf/black
rev: 22.12.0
hooks:
Expand All @@ -24,10 +25,6 @@ repos:
- 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
Copy link
Member Author

Choose a reason for hiding this comment

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

We could prob replace flake8 here with ruff too. I know ruff implements these plugins since I was using those as well.

    - pyproject-flake8==6.0.0
    - flake8-bugbear==22.12.6
    - flake8-typing-imports==1.14.0
    - flake8-docstrings==1.6.0

But haven't checked these two yet:

    - flake8-rst-docstrings==0.3.0
    - flake8-rst==0.8.0

rev: 6.0.0
hooks:
Expand Down Expand Up @@ -63,8 +60,3 @@ repos:
- id: codespell
stages: [commit, commit-msg]
args: [--ignore-words-list, 'titel,statics,ba,nd,te']
- repo: https://github.com/asottile/pyupgrade
rev: v3.3.1
hooks:
- id: pyupgrade
args: [--py38-plus]
13 changes: 6 additions & 7 deletions docs/dev/dev_install.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ You can install the basic functionality of atomate2 using pip:
pip install atomate2
```

If you are planing to use atomate2 with fireworks, you can install the optional
If you are planning to use atomate2 with fireworks, you can install the optional
fireworks components:

```bash
Expand Down Expand Up @@ -39,7 +39,7 @@ You can also install fireworks dependencies:
pip install .[fireworks]
```

Or do a developer install by using the ``-e`` flag:
Or do a developer install by using the `-e` flag:

```bash
pip install -e .
Expand All @@ -50,13 +50,13 @@ pip install -e .
If you're planning on contributing to the atomate2 source, you should also install
the developer requirements with:

```
```bash
pip install -e .[dev]
pre-commit install
```

The precommit command will ensure that changes to the source code match the
atomate2 style guidelines by running code linters such as `black`, `isort`,
The `pre-commit` command will ensure that changes to the source code match the
atomate2 style guidelines by running code linters such as `black`, `ruff`,
and `mypy` automatically with each commit.

## Running unit tests
Expand All @@ -76,8 +76,7 @@ pytest

## Building the documentation

The atomate2 documentation can be built using the sphinx package. First, install the
necessary requirement:
The atomate2 documentation can be built using the sphinx package. First, install the requirements:

```bash
pip install .[docs]
Expand Down
57 changes: 46 additions & 11 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ classifiers = [
"Topic :: Scientific/Engineering",
]
requires-python = ">=3.8"
dependencies =[
dependencies = [
"pymatgen>=2022.8.23",
"custodian>=2022.5.26",
"pydantic",
Expand All @@ -47,7 +47,7 @@ docs = [
"FireWorks==2.0.3",
"autodoc_pydantic==1.8.0",
"jupyter-book==0.13.1",
"jsonschema[format]"
"jsonschema[format]",
]
dev = ["pre-commit>=2.12.1"]
tests = ["pytest==7.2.1", "pytest-cov==4.0.0", "FireWorks==2.0.3"]
Expand Down Expand Up @@ -86,9 +86,6 @@ atomate2 = ["py.typed"]
method = "git"
default-tag = "0.0.1"

[tool.isort]
profile = "black"

[tool.flake8]
max-line-length = 88
max-doc-length = 88
Expand Down Expand Up @@ -128,9 +125,47 @@ exclude_lines = [
'^\s*@overload( |$)',
]

[tool.autoflake]
in-place = true
remove-all-unused-imports = true
remove-unused-variables = true
ignore-init-module-imports = true
expand-star-imports = true
[tool.ruff]
target-version = "py38"
select = [
"B", # flake8-bugbear
"C4", # flake8-comprehensions
"D", # pydocstyle
"E", # pycodestyle
"F", # pyflakes
"I", # isort
"PLE", # pylint error
"PLW", # pylint warning
"Q", # flake8-quotes
"RUF", # Ruff-specific rules
"SIM", # flake8-simplify
"TID", # tidy imports
"UP", # pyupgrade
"W", # pycodestyle
"YTT", # flake8-2020
]
ignore = [
"B019", # functools.lru_cache on methods can lead to memory leaks
"B023", # Function definition does not bind loop variable
"B904", # Within an except clause, raise exceptions with ...
"D100", # Missing docstring in public module
"D104", # Missing docstring in public package
"D105", # Missing docstring in magic method
"D107", # Missing docstring in __init__
"D200", # One-line docstring should fit on one line with quotes
"D205", # 1 blank line required between summary line and description
"D212", # Multi-line docstring summary should start at the first line
"D415", # First line should end with a period, question mark, or exclamation point
"E741", # tmp: we should fix all ambiguous variable names
"PLR2004", # Magic number
"PLW0120", # awaiting bug fix https://github.com/charliermarsh/ruff/issues/3019
"C408", # Unnecessary dict call - rewrite as a literal
"D416", # Section name should end with a colon
"SIM105", # Use contextlib.suppress(socket.gaierror, socket.herror) instead of try-except-pass
]
pydocstyle.convention = "google"
isort.known-first-party = ["atomate2"]

[tool.ruff.per-file-ignores]
"__init__.py" = ["F401"]
"**/tests/*" = ["D"]
2 changes: 1 addition & 1 deletion src/atomate2/amset/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class AmsetTaskDocument(StructureMetadata):
scattering_labels: List[str] = Field(
None, description="The scattering types used in the calculation"
)
soc: bool = Field(None, description="Whether spinorbit coupling was included")
soc: bool = Field(None, description="Whether spin-orbit coupling was included")
structure: Structure = Field(None, description="The structure used in this task")
_schema: str = Field(
__version__,
Expand Down
2 changes: 1 addition & 1 deletion src/atomate2/cli/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def vasp_test_data(test_dir):
[f" {v} -> {k}" for k, v in original_mapping.items()]
)

run_vasp_kwargs = {k: {"incar_settings": ["NSW", "ISMEAR"]} for k in mapping.keys()}
run_vasp_kwargs = {k: {"incar_settings": ["NSW", "ISMEAR"]} for k in mapping}
run_vasp_kwargs_str = pformat(run_vasp_kwargs).replace("\n", "\n ")

test_function_str = f"""Test files generated in test_data.
Expand Down
10 changes: 4 additions & 6 deletions src/atomate2/common/analysis/defects/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,11 +291,9 @@ def get_dQ(ref: Structure, distorted: Structure) -> float:
"""
return np.sqrt(
np.sum(
list(
map(
lambda x: x[0].distance(x[1]) ** 2 * x[0].specie.atomic_mass,
zip(ref, distorted),
)
)
[
x[0].distance(x[1]) ** 2 * x[0].specie.atomic_mass
for x in zip(ref, distorted)
]
)
)
6 changes: 1 addition & 5 deletions src/atomate2/common/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,11 +357,7 @@ def get_zfile(
found, then ``None`` will be returned.
"""
for file in directory_listing:
if base_name == file.name:
return file
elif base_name + ".gz" == file.name:
return file
elif base_name + ".GZ" == file.name:
if base_name in [file.name, file.name + ".gz", file.name + ".GZ"]:
return file

if allow_missing:
Expand Down
6 changes: 2 additions & 4 deletions src/atomate2/common/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ def retrieve_structure_from_materials_project(
and also the database version and specific task_id
corresponding to that Structure object also stored
"""

# inline import to avoid required dependency
try:
from mp_api.client import MPRester
Expand All @@ -132,14 +131,13 @@ def retrieve_structure_from_materials_project(

structure = doc.structure

if reset_magnetic_moments:
if reset_magnetic_moments and "magmom" in structure.site_properties:
# Materials Project stores magnetic moments via the `magmom` site property
# and we can safely assume that here. In general, since magnetic order
# can be represented in multiple ways such as Species.spin, the
# following method would be better:
# CollinearMagneticStructureAnalyzer.get_nonmagnetic_structure()
if "magmom" in structure.site_properties:
structure.remove_site_property("magmom")
structure.remove_site_property("magmom")

return Response(
output=structure,
Expand Down
7 changes: 3 additions & 4 deletions src/atomate2/common/schemas/cclib.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,9 @@ def from_logfile(
metadata["cpu_time"] = [str(m) for m in metadata["cpu_time"]]

# Get the final energy to store as its own key/value pair
if cclib_obj.scfenergies is not None:
energy = cclib_obj.scfenergies[-1]
else:
energy = None
energy = (
cclib_obj.scfenergies[-1] if cclib_obj.scfenergies is not None else None
)

# Now we construct the input molecule. Note that this is not necessarily
# the same as the initial molecule from the relaxation because the
Expand Down
2 changes: 1 addition & 1 deletion src/atomate2/common/schemas/elastic.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class FittingData(BaseModel):
None, description="The strains used to fit the elastic tensor."
)
pk_stresses: List[Matrix3D] = Field(
None, description="The PiolaKirchoff stresses used to fit the elastic tensor."
None, description="The Piola-Kirchoff stresses used to fit the elastic tensor."
)
deformations: List[Matrix3D] = Field(
None, description="The deformations corresponding to each strain state."
Expand Down
2 changes: 1 addition & 1 deletion src/atomate2/utils/file_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ def get_ssh_connection(
# try reading ssh config file
ssh_config = paramiko.SSHConfig().from_path(str(config_filename))

host_config = ssh_config.lookup(hostname)
host_config = ssh_config.lookup(hostname) # type: ignore[attr-defined]
for k in ("hostname", "user", "port"):
if k in host_config:
config[k.replace("user", "username")] = host_config[k]
Expand Down
2 changes: 1 addition & 1 deletion src/atomate2/vasp/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def copy_vasp_outputs(
directory_listing = file_client.listdir(src_dir, host=src_host)

# find required files
files = ("INCAR", "OUTCAR", "CONTCAR", "vasprun.xml") + tuple(additional_vasp_files)
files = ("INCAR", "OUTCAR", "CONTCAR", "vasprun.xml", *additional_vasp_files)
required_files = [get_zfile(directory_listing, r + relax_ext) for r in files]

# find optional files; do not fail if KPOINTS is missing, this might be KSPACING
Expand Down
12 changes: 12 additions & 0 deletions src/atomate2/vasp/flows/defect.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@

@dataclass
class ConfigurationCoordinateMaker(BaseCCDMaker):
"""Maker to calculate the configuration coordinate diagram.

Parameters
----------
name: str
The name of the flow created by this maker.
relax_maker: BaseVaspMaker
A maker to perform the relaxation of the defect.
static_maker: BaseVaspMaker
A maker to perform the static calculation of the defect.
"""

utf marked this conversation as resolved.
Show resolved Hide resolved
relax_maker: BaseVaspMaker = field(
default_factory=lambda: RelaxMaker(
input_set_generator=DEFECT_RELAX_GENERATOR,
Expand Down
5 changes: 1 addition & 4 deletions src/atomate2/vasp/jobs/phonons.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,7 @@ def get_supercell_size(
transformation.apply_transformation(structure=structure)

else:
if "max_atoms" not in kwargs:
max_atoms = 1000
else:
max_atoms = kwargs["max_atoms"]
max_atoms = 1000 if "max_atoms" not in kwargs else kwargs["max_atoms"]
if "angle_tolerance" not in kwargs:
kwargs["angle_tolerance"] = 1e-2
try:
Expand Down
2 changes: 1 addition & 1 deletion src/atomate2/vasp/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def run_vasp(
raise ValueError(f"Unsupported job type: {job_type}")

if wall_time is not None:
handlers = list(handlers) + [WalltimeHandler(wall_time=wall_time)]
handlers = [*handlers, WalltimeHandler(wall_time=wall_time)]

c = Custodian(
handlers,
Expand Down
5 changes: 1 addition & 4 deletions src/atomate2/vasp/schemas/calc_types/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ def run_type(vasp_parameters: Dict) -> RunType:
RunType
The run type.
"""
if vasp_parameters.get("LDAU", False):
is_hubbard = "+U"
else:
is_hubbard = ""
is_hubbard = "+U" if vasp_parameters.get("LDAU", False) else ""

def _variant_equal(v1, v2) -> bool:
"""Check two strings equal."""
Expand Down
2 changes: 1 addition & 1 deletion src/atomate2/vasp/schemas/calculation.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
]


_BADER_EXE_EXISTS = True if (which("bader") or which("bader.exe")) else False
_BADER_EXE_EXISTS = bool(which("bader") or which("bader.exe"))


class Status(ValueEnum):
Expand Down
Loading