Skip to content

fix(concat): convert --help gave non-zero exit status even when available #38

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

Merged
merged 5 commits into from
Dec 6, 2024
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
13 changes: 5 additions & 8 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
name: Tests

on:
- push
- pull_request

jobs:
build:
# timeout-minutes: 460
name: ${{ matrix.python-version }} / ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"]
os:
- ubuntu-latest
# - macos-latest
# - windows-latest # Haven't figured out how ImageMagick should be installed on win32 yet

steps:
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
Expand All @@ -26,21 +23,21 @@ jobs:
# You can test your matrix by printing the current Python version
- name: Display Python version
run: python -c "import sys; print(sys.version)"

- name: Install ImageMagick
uses: mfinelli/setup-imagemagick@v5
- name: Print ImageMagick version
run: magick --version
- name: Upgrade pip
run: |
pip install --upgrade pip
pip --version

- name: Install Poetry
run: |
pipx install poetry
poetry --version

- name: Install dependencies
run: |
poetry install

- name: Test with pytest
run: |
poetry run pytest
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ plot(7)
plot(8)
plot(9)
plot(10)
# See `convert -list font` for all available fonts.
# See `magick convert -list font` for all available fonts.
figs = [f"./assets/{i}.png" for i in range(1, 11)]
cosmoplots.combine(*figs).using(
font="JetBrainsMonoNL-NFM-Medium",
Expand Down
24 changes: 13 additions & 11 deletions cosmoplots/concat.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def using(
The position in the subfigure relative to `gravity`. Default is `(10.0,
10.0)`.
font : str, optional
The type of font to use, default is Times New Roman. See `convert -list
The type of font to use, default is Times New Roman. See `magick convert -list
font` for a list of available fonts.
fontsize : int, optional
The size of the font in pointsize. Default is to use the "font.size" field
Expand Down Expand Up @@ -194,7 +194,7 @@ def _check_params_before_save(
)
if self._ft in [".eps", ".pdf"]:
warnings.warn(
"The ImageMagick `convert` command does not work well with vector"
"The ImageMagick `magick convert` command does not work well with vector"
" formats. Consider combining the plots directly using matplotlib,"
" or change to a different format, such as 'png' or 'jpg'. See also"
" https://www.imagemagick.org/Usage/formats/#vector",
Expand All @@ -209,11 +209,12 @@ def _check_params_before_save(
@staticmethod
def _check_cli_available() -> None:
try:
subprocess.check_output("convert --help", shell=True)
subprocess.check_output("magick convert --help", shell=True)
except subprocess.CalledProcessError as e:
raise ChildProcessError(
"Calling `convert --help` did not work. Are you sure you have imagemagick installed?"
" If not, resort to the ImageMagick website: https://imagemagick.org/script/download.php"
"Calling `magick convert --help` did not work. Are you sure you have "
"imagemagick installed? If not, resort to the ImageMagick website: "
"https://imagemagick.org/script/download.php"
) from e

def _run_subprocess(self) -> None:
Expand All @@ -230,6 +231,7 @@ def _run_subprocess(self) -> None:
# Add label to images
subprocess.call(
[
"magick",
"convert",
"-units",
"PixelsPerInch",
Expand All @@ -253,14 +255,14 @@ def _run_subprocess(self) -> None:
# Choose first n items in the list
idx_sub = idx[j * self._w : (j + 1) * self._w]
subprocess.call(
["convert", "+append"]
["magick", "convert", "+append"]
+ [tmp_path / f"{str(i)}{self._ft}" for i in idx_sub]
+ [tmp_path / f"subfigure_{j}{self._ft}"]
)

# Create vertical subfigures from horizontal subfigures
subprocess.call(
["convert", "-append"]
["magick", "convert", "-append"]
+ [tmp_path / f"subfigure_{j}{self._ft}" for j in range(self._h)]
+ [self._output.resolve()]
)
Expand All @@ -273,7 +275,7 @@ def help(self) -> None:

def _conv_cmd(lab) -> str:
return (
f" convert in-{lab}{self._ft} -font {self._font} -pointsize"
f" magick convert in-{lab}{self._ft} -font {self._font} -pointsize"
f' {self._fontsize} -draw "gravity {self._gravity} fill {self._color}'
f" text {self._pos[0]},{self._pos[1]} '({lab})'\" {lab}{self._ft}\n"
)
Expand All @@ -285,10 +287,10 @@ def _conv_cmd(lab) -> str:
f"{_conv_cmd('c')}"
f"{_conv_cmd('d')}"
"Then to combine them horizontally:\n"
f" convert +append a{self._ft} b{self._ft} ab{self._ft}\n"
f" convert +append c{self._ft} d{self._ft} cd{self._ft}\n"
f" magick convert +append a{self._ft} b{self._ft} ab{self._ft}\n"
f" magick convert +append c{self._ft} d{self._ft} cd{self._ft}\n"
"And finally stack them vertically:\n"
f" convert -append ab{self._ft} cd{self._ft} out{self._ft}\n"
f" magick convert -append ab{self._ft} cd{self._ft} out{self._ft}\n"
"Optionally delete all temporary files:\n"
f" rm a{self._ft} b{self._ft} c{self._ft} d{self._ft} ab{self._ft} cd{self._ft}"
)
Expand Down
22 changes: 11 additions & 11 deletions tests/test_concat.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ def plot() -> None:
"""Create a simple plot."""
a = np.exp(np.linspace(-3, 5, 100))
fig = plt.figure()
ax = fig.add_subplot()
ax = fig.gca()
ax.set_xlabel("X Axis")
ax.set_ylabel("Y Axis")
ax.semilogy(a)


def test_convert_help() -> None:
"""Test that ImageMagick's convert command is available."""
result = subprocess.check_output(["convert", "--help"])
def test_magick_convert_help() -> None:
"""Test that ImageMagick's magick command is available."""
result = subprocess.check_output(["magick", "convert", "--help"])
out = "b'Version: ImageMagick'"
assert str(result[:20]) == out

Expand All @@ -39,15 +39,15 @@ def test_help(capfd) -> None:
out, err = capfd.readouterr()
help = (
"To create images with labels:\n"
" convert in-a.png -font Times-New-Roman -pointsize 100 -draw \"gravity northwest fill black text 10.0,10.0 '(a)'\" a.png\n"
" convert in-b.png -font Times-New-Roman -pointsize 100 -draw \"gravity northwest fill black text 10.0,10.0 '(b)'\" b.png\n"
" convert in-c.png -font Times-New-Roman -pointsize 100 -draw \"gravity northwest fill black text 10.0,10.0 '(c)'\" c.png\n"
" convert in-d.png -font Times-New-Roman -pointsize 100 -draw \"gravity northwest fill black text 10.0,10.0 '(d)'\" d.png\n"
" magick convert in-a.png -font Times-New-Roman -pointsize 100 -draw \"gravity northwest fill black text 10.0,10.0 '(a)'\" a.png\n"
" magick convert in-b.png -font Times-New-Roman -pointsize 100 -draw \"gravity northwest fill black text 10.0,10.0 '(b)'\" b.png\n"
" magick convert in-c.png -font Times-New-Roman -pointsize 100 -draw \"gravity northwest fill black text 10.0,10.0 '(c)'\" c.png\n"
" magick convert in-d.png -font Times-New-Roman -pointsize 100 -draw \"gravity northwest fill black text 10.0,10.0 '(d)'\" d.png\n"
"Then to combine them horizontally:\n"
" convert +append a.png b.png ab.png\n"
" convert +append c.png d.png cd.png\n"
" magick convert +append a.png b.png ab.png\n"
" magick convert +append c.png d.png cd.png\n"
"And finally stack them vertically:\n"
" convert -append ab.png cd.png out.png\n"
" magick convert -append ab.png cd.png out.png\n"
"Optionally delete all temporary files:\n"
" rm a.png b.png c.png d.png ab.png cd.png\n"
)
Expand Down
Loading