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.

Sign up for GitHub

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

Jump to bottom

Fix failing tests and migrate to importlib #216

Merged
merged 11 commits into from
Oct 11, 2023
Merged

Fix failing tests and migrate to importlib #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9b3f3b1
Move to importlib
oashour Jul 23, 2023
49af89e
Move from to importlib_resources for python 3.8
oashour Jul 23, 2023
10ddb82
Fix typo
oashour Jul 23, 2023
f9f226c
Move from pkg_resources to importlib
oashour Jul 23, 2023
052eb96
Turn tests into a package
oashour Jul 23, 2023
9936c01
Fix bug in ZnS path
oashour Jul 23, 2023
16e707a
Use importlib.resources if available
oashour Jul 23, 2023
47cc9f9
Move from pkg_resources to importlib
oashour Jul 23, 2023
d388cfc
Update tests workflow
oashour Jul 23, 2023
82d987c
Add back other python versions
oashour Jul 23, 2023
ae0c43b
Update docs workflow
oashour Jul 23, 2023
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
19 changes: 7 additions & 12 deletions .github/workflows/docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,19 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v3

- uses: actions/setup-python@v2
- uses: actions/setup-python@v4
with:
python-version: 3.9

- uses: actions/cache@v2
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip-${{ hashFiles('**/setup.py') }}
restore-keys: |
${{ runner.os }}-pip-
cache: pip
cache-dependency-path: setup.py

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install numpy
pip install -e .[docs]
python -m pip install --upgrade pip
python -m pip install numpy
python -m pip install -e '.[docs]'

- name: Build
run: sphinx-build docs/src docs_build
Expand Down
41 changes: 17 additions & 24 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,23 @@ jobs:

strategy:
matrix:
python-version: ['3.8', '3.9', '3.10']
python-version: ['3.8', '3.9', '3.10', '3.11']
name: Python ${{ matrix.python-version }}

steps:
- uses: actions/checkout@v2

- uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}

- uses: actions/cache@v2
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip-${{ hashFiles('**/setup.py') }}
restore-keys: |
${{ runner.os }}-pip-

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install numpy

pip install -e .
pip install -e .[tests]

- name: Test
run: pytest
- uses: actions/checkout@v3
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
cache: pip
cache-dependency-path: setup.py

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install numpy
python -m pip install -e '.[tests]'

- name: Test
run: pytest
3 changes: 2 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"Topic :: Scientific/Engineering :: Physics",
],
keywords="chemistry pymatgen dft vasp dos band",
packages=find_packages(),
packages=find_packages(exclude=["tests"]),
python_requires=">=3.8",
install_requires=[
"spglib",
Expand All @@ -42,6 +42,7 @@
"seekpath",
"castepxbin<1.0",
"colormath",
"importlib-resources",
],
extras_require={
"docs": ["sphinx", "sphinx-argparse"],
Expand Down
34 changes: 25 additions & 9 deletions sumo/cli/bandplot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@
import sys
import warnings

try:
from importlib.resources import files as ilr_files
except ImportError: # Python < 3.9
from importlib_resources import files as ilr_files
import matplotlib as mpl
from pkg_resources import Requirement, resource_filename
from pymatgen.electronic_structure.bandstructure import get_reconstructed_band_structure
from pymatgen.electronic_structure.bandstructure import (
get_reconstructed_band_structure,
)
from pymatgen.electronic_structure.core import Spin
from pymatgen.io.vasp.outputs import BSVasprun

Expand Down Expand Up @@ -339,6 +344,7 @@ def bandplot(

# currently not supported as it is a pain to make subplots within subplots,
# although need to check this is still the case
# FIXME: is this necessary if mode can only be "rgb" and "stacked"?
if "split" in mode and dos_file:
logging.error(
"ERROR: Plotting split projected band structure with DOS"
Expand Down Expand Up @@ -387,7 +393,9 @@ def bandplot(
else:
logging.info(f"Found PDOS file {pdos_file}")
else:
logging.info(f"Cell file {cell_file} does not exist, cannot plot PDOS.")
logging.info(
f"Cell file {cell_file} does not exist, cannot plot PDOS."
)

dos, pdos = read_castep_dos(
dos_file,
Expand Down Expand Up @@ -609,7 +617,8 @@ def _get_parser():
"-c",
"--code",
default="vasp",
help="Electronic structure code (default: vasp)." '"questaal" also supported.',
help="Electronic structure code (default: vasp)."
'"questaal" also supported.',
)
parser.add_argument(
"-p", "--prefix", metavar="P", help="prefix for the files generated"
Expand Down Expand Up @@ -750,7 +759,10 @@ def _get_parser():
"--orbitals",
type=_el_orb,
metavar="O",
help=("orbitals to split into lm-decomposed " 'contributions (e.g. "Ru.d")'),
help=(
"orbitals to split into lm-decomposed "
'contributions (e.g. "Ru.d")'
),
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what is going on with the linting here.

Obviously a human would never have written the existing version. Did some other auto-linter remove the line-break? And if so, did it break PEP8 in the process? @utf any idea?

This is why I dislike auto-formatters...

Copy link
Member

Choose a reason for hiding this comment

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

I checked the blame, and indeed this was ruined by black. The original version was

    parser.add_argument('--orbitals', type=_el_orb, metavar='O',
                        help=('orbitals to split into lm-decomposed '
                              'contributions (e.g. "Ru.d")'))

)
parser.add_argument(
"--atoms",
Expand Down Expand Up @@ -814,7 +826,9 @@ def _get_parser():
parser.add_argument(
"--height", type=float, default=None, help="height of the graph"
)
parser.add_argument("--width", type=float, default=None, help="width of the graph")
parser.add_argument(
"--width", type=float, default=None, help="width of the graph"
)
parser.add_argument(
"--ymin", type=float, default=-6.0, help="minimum energy on the y-axis"
)
Expand Down Expand Up @@ -865,16 +879,18 @@ def main():
logging.getLogger("").addHandler(console)

if args.config is None:
config_path = resource_filename(
Requirement.parse("sumo"), "sumo/plotting/orbital_colours.conf"
config_path = os.path.join(
ilr_files("sumo.plotting"), "orbital_colours.conf"
Copy link
Member

Choose a reason for hiding this comment

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

I think with importlib.resources.files we can have files("sumo.plotting") / "orbital_colours.conf" which is a bit cleaner.

)
else:
config_path = args.config
colours = configparser.ConfigParser()
colours.read(os.path.abspath(config_path))

warnings.filterwarnings("ignore", category=UserWarning, module="matplotlib")
warnings.filterwarnings("ignore", category=UnicodeWarning, module="matplotlib")
warnings.filterwarnings(
"ignore", category=UnicodeWarning, module="matplotlib"
)
warnings.filterwarnings("ignore", category=UserWarning, module="pymatgen")

bandplot(
Expand Down
51 changes: 39 additions & 12 deletions sumo/cli/dosplot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@

import matplotlib as mpl
import numpy as np
from pkg_resources import Requirement, resource_filename
try:
from importlib.resources import files as ilr_files
except ImportError: # Python < 3.9
from importlib_resources import files as ilr_files

mpl.use("Agg")

Expand Down Expand Up @@ -205,7 +208,6 @@ def dosplot(
)

elif code.lower() == "castep":

if filename:
bands_file = filename
else:
Expand Down Expand Up @@ -261,7 +263,13 @@ def dosplot(
else:
pdos_candidates = glob("dos.*")
for candidate in pdos_candidates:
if candidate.split(".")[-1] in ("pdf", "png", "svg", "jpg", "jpeg"):
if candidate.split(".")[-1] in (
"pdf",
"png",
"svg",
"jpg",
"jpeg",
):
continue
elif candidate.split(".")[-1].lower() in ("gz", "z", "bz2"):
pdos_file = candidate
Expand Down Expand Up @@ -422,14 +430,20 @@ def _get_parser():
)

parser.add_argument(
"-f", "--filename", help="vasprun.xml file to plot", default=None, metavar="F"
"-f",
"--filename",
help="vasprun.xml file to plot",
default=None,
metavar="F",
)
parser.add_argument(
"-c",
"--code",
default="vasp",
metavar="C",
help=('Input file format: "vasp" (vasprun.xml) or ' '"questaal" (opt.ext)'),
help=(
'Input file format: "vasp" (vasprun.xml) or ' '"questaal" (opt.ext)'
Copy link
Member

Choose a reason for hiding this comment

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

More absurdity from linters. This should be a single string.

),
)
parser.add_argument(
"-p", "--prefix", metavar="P", help="prefix for the files generated"
Expand All @@ -449,7 +463,10 @@ def _get_parser():
"--orbitals",
type=_el_orb,
metavar="O",
help=("orbitals to split into lm-decomposed " 'contributions (e.g. "Ru.d")'),
help=(
"orbitals to split into lm-decomposed "
'contributions (e.g. "Ru.d")'
),
)
parser.add_argument(
"-a",
Expand Down Expand Up @@ -500,7 +517,10 @@ def _get_parser():
),
)
parser.add_argument(
"--no-legend", action="store_false", dest="legend", help="hide the plot legend"
"--no-legend",
action="store_false",
dest="legend",
help="hide the plot legend",
)
parser.add_argument(
"--legend-frame",
Expand Down Expand Up @@ -540,7 +560,9 @@ def _get_parser():
parser.add_argument(
"--height", type=float, default=None, help="height of the graph"
)
parser.add_argument("--width", type=float, default=None, help="width of the graph")
parser.add_argument(
"--width", type=float, default=None, help="width of the graph"
)
parser.add_argument(
"--xmin", type=float, default=-6.0, help="minimum energy on the x-axis"
)
Expand Down Expand Up @@ -570,7 +592,10 @@ def _get_parser():
help="x-axis (i.e. energy) label/units",
)
parser.add_argument(
"--ylabel", type=str, default="DOS", help="y-axis (i.e. DOS) label/units"
"--ylabel",
type=str,
default="DOS",
help="y-axis (i.e. DOS) label/units",
)
parser.add_argument(
"--yscale", type=float, default=1, help="scaling factor for the y-axis"
Expand Down Expand Up @@ -609,16 +634,18 @@ def main():
logging.getLogger("").addHandler(console)

if args.config is None:
config_path = resource_filename(
Requirement.parse("sumo"), "sumo/plotting/orbital_colours.conf"
config_path = os.path.join(
ilr_files("sumo.plotting"), "orbital_colours.conf"
Copy link
Member

Choose a reason for hiding this comment

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

Again, I think the join can become a /.

)
else:
config_path = args.config
colours = configparser.ConfigParser()
colours.read(os.path.abspath(config_path))

warnings.filterwarnings("ignore", category=UserWarning, module="matplotlib")
warnings.filterwarnings("ignore", category=UnicodeWarning, module="matplotlib")
warnings.filterwarnings(
"ignore", category=UnicodeWarning, module="matplotlib"
)
warnings.filterwarnings("ignore", category=UserWarning, module="pymatgen")

if args.zero_energy is not None:
Expand Down
Loading