From 6312462a9986697e8334ca39f274694958a7a3ba Mon Sep 17 00:00:00 2001 From: Aditya Vaidya Date: Mon, 26 Oct 2026 15:56:16 -0700 Subject: [PATCH 1/3] REF move Inkscape helpers to cortex.dependencies Look up Inkscape lazily, so installing it after import is picked up. --- cortex/{testing_utils.py => dependencies.py} | 42 ++++++------ cortex/svgoverlay.py | 10 +-- cortex/tests/test_dataset.py | 4 +- ..._testing_utils.py => test_dependencies.py} | 65 ++++++++++++++----- cortex/tests/test_nan_alpha.py | 2 +- cortex/tests/test_quickflat.py | 4 +- cortex/utils.py | 6 +- 7 files changed, 84 insertions(+), 49 deletions(-) rename cortex/{testing_utils.py => dependencies.py} (67%) rename cortex/tests/{test_testing_utils.py => test_dependencies.py} (57%) diff --git a/cortex/testing_utils.py b/cortex/dependencies.py similarity index 67% rename from cortex/testing_utils.py rename to cortex/dependencies.py index a987cb6cd..cb779aa7c 100644 --- a/cortex/testing_utils.py +++ b/cortex/dependencies.py @@ -1,19 +1,21 @@ -"""Module containing utils for testing""" +"""Locate external tools (e.g. Inkscape) configured under ``[dependency_paths]``.""" +from __future__ import annotations + import os import subprocess as sp +from functools import cache from shutil import which -from . import options -import os +from . import options from .options import config -def has_installed(name): +def has_installed(name: str) -> bool: binary_file = options.config.get('dependency_paths', name) return os.path.exists(binary_file) or which(name) is not None -def inkscapePath(): +def inkscape_path() -> str | None: """Return the path of the inkscape executable to run, or None. The 'inkscape' option of the 'dependency_paths' config section is used @@ -30,16 +32,25 @@ def inkscapePath(): return which('inkscape') -def inkscape_version(): - inkscapeExecutable = inkscapePath() +def inkscape_version() -> str | None: + """Return the version string of the inkscape executable, or None. + + Nothing is cached while inkscape is missing, so installing it after + pycortex has been imported takes effect on the next call. + """ + inkscapeExecutable = inkscape_path() if inkscapeExecutable is None: return None + return _query_inkscape_version(inkscapeExecutable) + + +@cache +def _query_inkscape_version(inkscapeExecutable: str) -> str | None: result = sp.run([inkscapeExecutable, '--version'], stdout = sp.PIPE, stderr = sp.PIPE, check = True) # Combine stdout and stderr; some systems print diagnostic messages # (e.g. "Setting _INKSCAPE_GC=disable …") before the version line. - combined = result.stdout + result.stderr - if isinstance(combined, bytes): - combined = combined.decode('utf-8') + raw = result.stdout + result.stderr + combined = raw.decode('utf-8') if isinstance(raw, bytes) else raw # Find the line that starts with 'Inkscape' to get the real version, # e.g. 'Inkscape 1.2.2 (b0a8486, 2022-12-01)' for line in combined.splitlines(): @@ -47,14 +58,3 @@ def inkscape_version(): version = line.split()[1] return version return None - - -INKSCAPE_PATH = inkscapePath() -INKSCAPE_VERSION = inkscape_version() -if INKSCAPE_VERSION is None: - print(("Inkscape not found. Some tests will be skipped. " - "If you are on a Mac or Windows machine, please specify " - "the path to the Inkscape binary in the config file " - f"(Here: {options.usercfg}).")) - - diff --git a/cortex/svgoverlay.py b/cortex/svgoverlay.py index b6ce59480..c24f603e5 100644 --- a/cortex/svgoverlay.py +++ b/cortex/svgoverlay.py @@ -16,7 +16,7 @@ from lxml.builder import E from .options import config -from .testing_utils import INKSCAPE_PATH, INKSCAPE_VERSION +from .dependencies import inkscape_path, inkscape_version svgns = "http://www.w3.org/2000/svg" inkns = "http://www.inkscape.org/namespaces/inkscape" @@ -229,7 +229,7 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru """ # Give a more informative error in case we don't have inkscape # installed - if INKSCAPE_VERSION is None: + if inkscape_version() is None: raise RuntimeError( "Inkscape doesn't seem to be installed on this system." "SVGOverlay.get_texture requires inkscape." @@ -293,11 +293,11 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru svgtemp.close() svgfile = svgtemp.name - inkscape_cmd = INKSCAPE_PATH + inkscape_cmd = inkscape_path() # The command is built as an argument list rather than as a string that # is then split, so that a configured inkscape path containing spaces # or backslashes is passed through unchanged. - if LooseVersion(INKSCAPE_VERSION) < LooseVersion('1.0'): + if LooseVersion(inkscape_version()) < LooseVersion('1.0'): cmd = [inkscape_cmd, '-z', '-h', str(height), '-e', pngfile, svgfile] else: cmd = [inkscape_cmd, '-h', str(height), '--export-filename', pngfile, svgfile] @@ -326,7 +326,7 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru except SyntaxError as e: os.unlink(pngfile) raise RuntimeError(f"Error reading image from {pngfile}: {e}" - f" (inkscape version: {INKSCAPE_VERSION})" + f" (inkscape version: {inkscape_version()})" f" (inkscape command: {inkscape_cmd})" f" (stdout: {stdout})" f" (stderr: {stderr})") diff --git a/cortex/tests/test_dataset.py b/cortex/tests/test_dataset.py index 53bae0674..9a062a59f 100644 --- a/cortex/tests/test_dataset.py +++ b/cortex/tests/test_dataset.py @@ -4,11 +4,11 @@ import pytest from cortex import db, dataset -from cortex.testing_utils import inkscapePath +from cortex.dependencies import inkscape_path subj, xfmname, nverts, volshape = "S1", "fullhead", 304380, (31, 100, 100) -no_inkscape = inkscapePath() is None +no_inkscape = inkscape_path() is None def test_braindata(): diff --git a/cortex/tests/test_testing_utils.py b/cortex/tests/test_dependencies.py similarity index 57% rename from cortex/tests/test_testing_utils.py rename to cortex/tests/test_dependencies.py index 930dd4bb2..1110c2a5e 100644 --- a/cortex/tests/test_testing_utils.py +++ b/cortex/tests/test_dependencies.py @@ -1,10 +1,21 @@ -"""Tests for cortex/testing_utils.py""" +"""Tests for cortex/dependencies.py""" import subprocess from unittest import mock import pytest -from cortex.testing_utils import inkscape_version +from cortex.dependencies import ( + _query_inkscape_version, + inkscape_path, + inkscape_version, +) + + +@pytest.fixture(autouse=True) +def _clear_cache(): + _query_inkscape_version.cache_clear() + yield + _query_inkscape_version.cache_clear() def _make_run_result(stdout=b'', stderr=b'', returncode=0): @@ -16,14 +27,14 @@ def _make_run_result(stdout=b'', stderr=b'', returncode=0): return result -@mock.patch('cortex.testing_utils.inkscapePath', return_value=None) +@mock.patch('cortex.dependencies.inkscape_path', return_value=None) def test_inkscape_version_not_installed(mock_path): """Returns None when inkscape is neither configured nor on PATH.""" assert inkscape_version() is None -@mock.patch('cortex.testing_utils.inkscapePath', return_value='inkscape') -@mock.patch('cortex.testing_utils.sp.run') +@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape') +@mock.patch('cortex.dependencies.sp.run') def test_inkscape_version_clean_stdout(mock_run, mock_path): """Parses version correctly from clean stdout output.""" mock_run.return_value = _make_run_result( @@ -32,8 +43,8 @@ def test_inkscape_version_clean_stdout(mock_run, mock_path): assert inkscape_version() == '1.0' -@mock.patch('cortex.testing_utils.inkscapePath', return_value='inkscape') -@mock.patch('cortex.testing_utils.sp.run') +@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape') +@mock.patch('cortex.dependencies.sp.run') def test_inkscape_version_newer(mock_run, mock_path): """Parses a newer version number correctly.""" mock_run.return_value = _make_run_result( @@ -42,8 +53,8 @@ def test_inkscape_version_newer(mock_run, mock_path): assert inkscape_version() == '1.2.2' -@mock.patch('cortex.testing_utils.inkscapePath', return_value='inkscape') -@mock.patch('cortex.testing_utils.sp.run') +@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape') +@mock.patch('cortex.dependencies.sp.run') def test_inkscape_version_with_diagnostic_noise(mock_run, mock_path): """Returns correct version even when a diagnostic message precedes it. @@ -60,8 +71,8 @@ def test_inkscape_version_with_diagnostic_noise(mock_run, mock_path): assert inkscape_version() == '1.2.2' -@mock.patch('cortex.testing_utils.inkscapePath', return_value='inkscape') -@mock.patch('cortex.testing_utils.sp.run') +@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape') +@mock.patch('cortex.dependencies.sp.run') def test_inkscape_version_noise_in_stderr(mock_run, mock_path): """Returns correct version when the diagnostic message is on stderr.""" mock_run.return_value = _make_run_result( @@ -71,18 +82,42 @@ def test_inkscape_version_noise_in_stderr(mock_run, mock_path): assert inkscape_version() == '1.2.2' -@mock.patch('cortex.testing_utils.inkscapePath', return_value='inkscape') -@mock.patch('cortex.testing_utils.sp.run') +@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape') +@mock.patch('cortex.dependencies.sp.run') def test_inkscape_version_no_inkscape_line(mock_run, mock_path): """Returns None when no 'Inkscape …' line is found in output.""" mock_run.return_value = _make_run_result(stdout=b'some unexpected output\n') assert inkscape_version() is None -@mock.patch('cortex.testing_utils.inkscapePath', return_value='inkscape') -@mock.patch('cortex.testing_utils.sp.run') +@mock.patch('cortex.dependencies.inkscape_path', return_value='inkscape') +@mock.patch('cortex.dependencies.sp.run') def test_inkscape_version_subprocess_error(mock_run, mock_path): """Propagates CalledProcessError when inkscape exits non-zero.""" mock_run.side_effect = subprocess.CalledProcessError(1, 'inkscape') with pytest.raises(subprocess.CalledProcessError): inkscape_version() + + +@mock.patch('cortex.dependencies.sp.run') +@mock.patch('cortex.dependencies.which') +@mock.patch('cortex.dependencies.config') +def test_inkscape_lookup_retries_until_found(mock_config, mock_which, mock_run): + """A failed lookup is not cached, so a later install is picked up.""" + mock_config.get.return_value = '' + mock_which.return_value = None + assert inkscape_path() is None + assert inkscape_version() is None + + # Inkscape gets installed while the process is still running. + mock_which.return_value = '/usr/bin/inkscape' + mock_run.return_value = _make_run_result( + stdout=b'Inkscape 1.2.2 (b0a8486, 2022-12-01)\n' + ) + assert inkscape_path() == '/usr/bin/inkscape' + assert inkscape_version() == '1.2.2' + + # Once found, the version is cached and the subprocess is not run again. + mock_run.reset_mock() + assert inkscape_version() == '1.2.2' + mock_run.assert_not_called() diff --git a/cortex/tests/test_nan_alpha.py b/cortex/tests/test_nan_alpha.py index a1f9d770b..2d6e80bd4 100644 --- a/cortex/tests/test_nan_alpha.py +++ b/cortex/tests/test_nan_alpha.py @@ -19,7 +19,7 @@ import cortex from cortex import dataset -from cortex.testing_utils import has_installed +from cortex.dependencies import has_installed import cortex.utils from cortex.webgl.data import Package from cortex.webgl.serve import NPEncode diff --git a/cortex/tests/test_quickflat.py b/cortex/tests/test_quickflat.py index 3ab9e986e..809f7cb0d 100644 --- a/cortex/tests/test_quickflat.py +++ b/cortex/tests/test_quickflat.py @@ -7,10 +7,10 @@ from cortex import dataset import cortex.quickflat.utils # for ty from cortex.svgoverlay import svgns -from cortex.testing_utils import inkscapePath +from cortex.dependencies import inkscape_path from cortex.webgl.data import Package -no_inkscape = inkscapePath() is None +no_inkscape = inkscape_path() is None def random_volume(with_nan=False, **kwargs): diff --git a/cortex/utils.py b/cortex/utils.py index dcfe5ccb5..e2094e71d 100644 --- a/cortex/utils.py +++ b/cortex/utils.py @@ -27,7 +27,7 @@ from .freesurfer import fs_aseg_dict from .options import config from .polyutils import Surface -from .testing_utils import INKSCAPE_PATH, INKSCAPE_VERSION +from .dependencies import inkscape_path, inkscape_version from .volume import anat2epispace # register_cmap is deprecated in matplotlib > 3.7.0 and replaced by colormaps.register @@ -511,8 +511,8 @@ def add_roi(data, name="new_roi", open_inkscape=True, add_path=True, svg.rois.add_shape(name, binascii.b2a_base64(fp.read()).decode('utf-8'), add_path) if open_inkscape: - inkscape_cmd = INKSCAPE_PATH - if LooseVersion(INKSCAPE_VERSION) < LooseVersion('1.0'): + inkscape_cmd = inkscape_path() + if LooseVersion(inkscape_version()) < LooseVersion('1.0'): cmd = [inkscape_cmd, '-f', svg.svgfile] else: cmd = [inkscape_cmd, svg.svgfile] From b29fb28165d336549929203c65ee30c80fe2accd Mon Sep 17 00:00:00 2001 From: Aditya Vaidya Date: Mon, 26 Oct 2026 15:56:16 -0700 Subject: [PATCH 2/3] REF condense webgl show() startup messages into one line --- cortex/webgl/view.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/cortex/webgl/view.py b/cortex/webgl/view.py index 2d0e04850..99b3b3941 100644 --- a/cortex/webgl/view.py +++ b/cortex/webgl/view.py @@ -1634,20 +1634,14 @@ def get_local_client(self): server.start() local_url, network_url = _viewer_urls(server.port) - print("Started server on port %d"%server.port) - if network_url == local_url: - print("Open the viewer at %s"%local_url) - else: - print("Open the viewer at %s (from another machine: %s)" - %(local_url, network_url)) + message = "Started server on port %d, open the viewer at %s"%(server.port, local_url) + if network_url != local_url: + message += " (from another machine: %s)"%network_url + print(message) if display_url and not open_browser: try: from IPython.display import HTML, display - link = 'Open viewer: {0}'.format(local_url) - if network_url != local_url: - link += (' (from another machine: ' - '{0})'.format(network_url)) - display(HTML(link)) + display(HTML('Open viewer: {0}'.format(local_url))) except: pass From 1505fa9dc52c59099231acdd35e6d863c70628f4 Mon Sep 17 00:00:00 2001 From: Aditya Vaidya Date: Mon, 26 Oct 2026 16:34:37 -0700 Subject: [PATCH 3/3] REF add require_inkscape and use it in get_texture and add_roi Fail early with a hint to set [dependency_paths], and look up the path and version once per call. --- cortex/dependencies.py | 13 +++++++++++++ cortex/svgoverlay.py | 14 ++++---------- cortex/tests/test_dependencies.py | 18 ++++++++++++++++++ cortex/utils.py | 9 ++++++--- 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/cortex/dependencies.py b/cortex/dependencies.py index cb779aa7c..7c0ebee05 100644 --- a/cortex/dependencies.py +++ b/cortex/dependencies.py @@ -44,6 +44,19 @@ def inkscape_version() -> str | None: return _query_inkscape_version(inkscapeExecutable) +def require_inkscape() -> tuple[str, str]: + """Return the (path, version) of inkscape, or raise if it is not available.""" + path = inkscape_path() + version = inkscape_version() + if path is None or version is None: + raise RuntimeError( + "Inkscape doesn't seem to be installed on this system, and is " + "required here. Please make sure that inkscape is installed and " + "accessible from the terminal, or set the 'inkscape' option in " + f"the [dependency_paths] section of {options.usercfg}.") + return path, version + + @cache def _query_inkscape_version(inkscapeExecutable: str) -> str | None: result = sp.run([inkscapeExecutable, '--version'], stdout = sp.PIPE, stderr = sp.PIPE, check = True) diff --git a/cortex/svgoverlay.py b/cortex/svgoverlay.py index c24f603e5..ede70d912 100644 --- a/cortex/svgoverlay.py +++ b/cortex/svgoverlay.py @@ -16,7 +16,7 @@ from lxml.builder import E from .options import config -from .dependencies import inkscape_path, inkscape_version +from .dependencies import require_inkscape svgns = "http://www.w3.org/2000/svg" inkns = "http://www.inkscape.org/namespaces/inkscape" @@ -229,12 +229,7 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru """ # Give a more informative error in case we don't have inkscape # installed - if inkscape_version() is None: - raise RuntimeError( - "Inkscape doesn't seem to be installed on this system." - "SVGOverlay.get_texture requires inkscape." - "Please make sure that inkscape is installed and that is " - "accessible from the terminal.") + inkscape_cmd, inkscape_ver = require_inkscape() if shadow is not None: for blur in self.svg.findall(".//{%s}feGaussianBlur"%svgns): @@ -293,11 +288,10 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru svgtemp.close() svgfile = svgtemp.name - inkscape_cmd = inkscape_path() # The command is built as an argument list rather than as a string that # is then split, so that a configured inkscape path containing spaces # or backslashes is passed through unchanged. - if LooseVersion(inkscape_version()) < LooseVersion('1.0'): + if LooseVersion(inkscape_ver) < LooseVersion('1.0'): cmd = [inkscape_cmd, '-z', '-h', str(height), '-e', pngfile, svgfile] else: cmd = [inkscape_cmd, '-h', str(height), '--export-filename', pngfile, svgfile] @@ -326,7 +320,7 @@ def get_texture(self, layer_name, height, name=None, background=None, labels=Tru except SyntaxError as e: os.unlink(pngfile) raise RuntimeError(f"Error reading image from {pngfile}: {e}" - f" (inkscape version: {inkscape_version()})" + f" (inkscape version: {inkscape_ver})" f" (inkscape command: {inkscape_cmd})" f" (stdout: {stdout})" f" (stderr: {stderr})") diff --git a/cortex/tests/test_dependencies.py b/cortex/tests/test_dependencies.py index 1110c2a5e..7cb28e0d1 100644 --- a/cortex/tests/test_dependencies.py +++ b/cortex/tests/test_dependencies.py @@ -8,6 +8,7 @@ _query_inkscape_version, inkscape_path, inkscape_version, + require_inkscape, ) @@ -121,3 +122,20 @@ def test_inkscape_lookup_retries_until_found(mock_config, mock_which, mock_run): mock_run.reset_mock() assert inkscape_version() == '1.2.2' mock_run.assert_not_called() + + +@mock.patch('cortex.dependencies.inkscape_path', return_value=None) +def test_require_inkscape_missing(mock_path): + """Raises an error that points at the config option when inkscape is missing.""" + with pytest.raises(RuntimeError, match=r'\[dependency_paths\]'): + require_inkscape() + + +@mock.patch('cortex.dependencies.inkscape_path', return_value='/usr/bin/inkscape') +@mock.patch('cortex.dependencies.sp.run') +def test_require_inkscape_found(mock_run, mock_path): + """Returns the path and version together when inkscape is available.""" + mock_run.return_value = _make_run_result( + stdout=b'Inkscape 1.2.2 (b0a8486, 2022-12-01)\n' + ) + assert require_inkscape() == ('/usr/bin/inkscape', '1.2.2') diff --git a/cortex/utils.py b/cortex/utils.py index e2094e71d..ad9b7bdfb 100644 --- a/cortex/utils.py +++ b/cortex/utils.py @@ -27,7 +27,7 @@ from .freesurfer import fs_aseg_dict from .options import config from .polyutils import Surface -from .dependencies import inkscape_path, inkscape_version +from .dependencies import require_inkscape from .volume import anat2epispace # register_cmap is deprecated in matplotlib > 3.7.0 and replaced by colormaps.register @@ -499,6 +499,10 @@ def add_roi(data, name="new_roi", open_inkscape=True, add_path=True, from . import dataset, quickflat + # Fail before touching the overlay file if Inkscape is needed but missing. + if open_inkscape: + inkscape_cmd, inkscape_ver = require_inkscape() + dv = dataset.normalize(data) if isinstance(dv, dataset.Dataset): raise TypeError("Please specify a data view") @@ -511,8 +515,7 @@ def add_roi(data, name="new_roi", open_inkscape=True, add_path=True, svg.rois.add_shape(name, binascii.b2a_base64(fp.read()).decode('utf-8'), add_path) if open_inkscape: - inkscape_cmd = inkscape_path() - if LooseVersion(inkscape_version()) < LooseVersion('1.0'): + if LooseVersion(inkscape_ver) < LooseVersion('1.0'): cmd = [inkscape_cmd, '-f', svg.svgfile] else: cmd = [inkscape_cmd, svg.svgfile]