From a84644d5f5c1ea65f2fefd553797ef604182e818 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Thu, 24 Jul 2025 16:19:00 -0700 Subject: [PATCH 01/41] Remove python 2 compat junk --- doc/api/idents.rst | 1 - lib/python/pyflyby/_idents.py | 10 ++-------- lib/python/pyflyby/_modules.py | 8 ++++++++ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/doc/api/idents.rst b/doc/api/idents.rst index 7ce0170f..bd4bf3a2 100644 --- a/doc/api/idents.rst +++ b/doc/api/idents.rst @@ -2,4 +2,3 @@ _idents module ============== .. automodule:: pyflyby._idents :members: - :exclude-members: _my_iskeyword \ No newline at end of file diff --git a/lib/python/pyflyby/_idents.py b/lib/python/pyflyby/_idents.py index 7c1ca72f..d086be49 100644 --- a/lib/python/pyflyby/_idents.py +++ b/lib/python/pyflyby/_idents.py @@ -5,7 +5,7 @@ from functools import total_ordering -from keyword import kwlist +from keyword import iskeyword import re from pyflyby._util import cached_attribute, cmp @@ -13,12 +13,6 @@ from typing import Optional, Tuple, Dict -# Don't consider "print" a keyword, in order to be compatible with user code -# that uses "from __future__ import print_function". -_my_kwlist = list(kwlist) -_my_iskeyword = frozenset(_my_kwlist).__contains__ - - # TODO: use DottedIdentifier.prefixes def dotted_prefixes(dotted_name, reverse=False): """ @@ -114,7 +108,7 @@ def is_identifier(s: str, dotted: bool = False, prefix: bool = False): return is_identifier(s + '_', dotted=dotted, prefix=False) if dotted: return all(is_identifier(w, dotted=False) for w in s.split('.')) - return s.isidentifier() and not _my_iskeyword(s) + return s.isidentifier() and not iskeyword(s) def brace_identifiers(text): diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 4a1b24a8..98b2c57e 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -307,6 +307,14 @@ def list(): # Canonicalize. return tuple(ModuleHandle(m) for m in sorted(set(module_names))) + # with ExcludeImplicitCwdFromPathCtx(): + # modules = [] + # for mod in sorted(set(pkgutil.iter_modules(None)), key=lambda x: x[1]): + # name = mod[0] + # if is_identifier(name): + # modules.append(ModuleHandle(name)) + # return modules + @cached_property def submodules(self): """ From 6a283ca9bb8ce0865ef270d1c877f2830699c3eb Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 25 Jul 2025 09:41:49 -0700 Subject: [PATCH 02/41] WIP --- lib/python/pyflyby/_interactive.py | 9 ++++++--- lib/python/pyflyby/_modules.py | 30 +++++++++++++----------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/lib/python/pyflyby/_interactive.py b/lib/python/pyflyby/_interactive.py index d523f589..31c8d3d3 100644 --- a/lib/python/pyflyby/_interactive.py +++ b/lib/python/pyflyby/_interactive.py @@ -22,7 +22,7 @@ auto_import, clear_failed_imports_cache, load_symbol) -from pyflyby._dynimp import (inject as inject_dynamic_import, +from pyflyby._dynimp import (inject as inject_dynamic_import, PYFLYBY_LAZY_LOAD_PREFIX) from pyflyby._comms import (initialize_comms, remove_comms, send_comm_message, MISSING_IMPORTS) @@ -837,9 +837,12 @@ def complete_symbol(fullname, namespaces, db=None, autoimported=None, ip=None, if '.' not in name: results.add(name) results.update(known.member_names.get("", [])) - results.update([str(m) for m in ModuleHandle.list()]) - assert all('.' not in r for r in results) + + results.update(ModuleHandle.list()) + # results.update([str(m) for m in ModuleHandle.list()]) + # assert all('.' not in r for r in results) results = sorted([r for r in results if r.startswith(attrname)]) + elif len(splt) == 2: # Check members, including known sub-modules and importable sub-modules. pname = splt[0] diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 98b2c57e..49162987 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -288,6 +288,7 @@ def list(): ``tuple`` of `ModuleHandle` s """ import pkgutil + # Get the list of top-level packages/modules using pkgutil. # We exclude "." from sys.path while doing so. Python includes "." in # sys.path by default, but this is undesirable for autoimporting. If @@ -296,24 +297,19 @@ def list(): # working directory is /tmp, trying to enumerate modules there also # causes problems, because there are typically directories there not # readable by the current user. - with ExcludeImplicitCwdFromPathCtx(): - modlist = pkgutil.iter_modules(None) - module_names = [t[1] for t in modlist] - # pkgutil includes all *.py even if the name isn't a legal python - # module name, e.g. if a directory in $PYTHONPATH has files named - # "try.py" or "123.py", pkgutil will return entries named "try" or - # "123". Filter those out. - module_names = [m for m in module_names if is_identifier(m)] - # Canonicalize. - return tuple(ModuleHandle(m) for m in sorted(set(module_names))) - # with ExcludeImplicitCwdFromPathCtx(): - # modules = [] - # for mod in sorted(set(pkgutil.iter_modules(None)), key=lambda x: x[1]): - # name = mod[0] - # if is_identifier(name): - # modules.append(ModuleHandle(name)) - # return modules + # modlist = pkgutil.iter_modules(None) + # module_names = [t[1] for t in modlist] + # # pkgutil includes all *.py even if the name isn't a legal python + # # module name, e.g. if a directory in $PYTHONPATH has files named + # # "try.py" or "123.py", pkgutil will return entries named "try" or + # # "123". Filter those out. + # module_names = [m for m in module_names if is_identifier(m)] + # # Canonicalize. + # return tuple(ModuleHandle(m) for m in sorted(set(module_names))) + + with ExcludeImplicitCwdFromPathCtx(): + return [mod.name for mod in pkgutil.iter_modules() if is_identifier(mod.name)] @cached_property def submodules(self): From 1eb0634b97aef83a749ecec3142623e06fa77b92 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 25 Jul 2025 12:39:30 -0700 Subject: [PATCH 03/41] Cut as many corners as possible while still remaining with python --- lib/python/pyflyby/_interactive.py | 3 - lib/python/pyflyby/_modules.py | 133 +++++++++++++++++++++++------ 2 files changed, 106 insertions(+), 30 deletions(-) diff --git a/lib/python/pyflyby/_interactive.py b/lib/python/pyflyby/_interactive.py index 31c8d3d3..fe45d54d 100644 --- a/lib/python/pyflyby/_interactive.py +++ b/lib/python/pyflyby/_interactive.py @@ -837,10 +837,7 @@ def complete_symbol(fullname, namespaces, db=None, autoimported=None, ip=None, if '.' not in name: results.add(name) results.update(known.member_names.get("", [])) - results.update(ModuleHandle.list()) - # results.update([str(m) for m in ModuleHandle.list()]) - # assert all('.' not in r for r in results) results = sorted([r for r in results if r.startswith(attrname)]) elif len(splt) == 2: diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 49162987..5320bf98 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -6,8 +6,10 @@ import ast from functools import cached_property, total_ordering +import importlib import itertools import os +import pkgutil from pyflyby._file import FileText, Filename from pyflyby._idents import DottedIdentifier, is_identifier @@ -19,7 +21,7 @@ from six import reraise import sys import types -from typing import Any, Dict +from typing import Any, Dict, Generator class ErrorDuringImportError(ImportError): """ @@ -280,36 +282,20 @@ def block(self): @staticmethod @memoize - def list(): - """ - Enumerate all top-level packages/modules. + def list() -> list[str]: + """Enumerate all top-level packages/modules. - :rtype: - ``tuple`` of `ModuleHandle` s - """ - import pkgutil + The current working directory is excluded for autoimporting; if we autoimported + random python scripts in the current directory, we could accidentally execute + code with side effects. - # Get the list of top-level packages/modules using pkgutil. - # We exclude "." from sys.path while doing so. Python includes "." in - # sys.path by default, but this is undesirable for autoimporting. If - # we autoimported random python scripts in the current directory, we - # could accidentally execute code with side effects. If the current - # working directory is /tmp, trying to enumerate modules there also - # causes problems, because there are typically directories there not - # readable by the current user. - # with ExcludeImplicitCwdFromPathCtx(): - # modlist = pkgutil.iter_modules(None) - # module_names = [t[1] for t in modlist] - # # pkgutil includes all *.py even if the name isn't a legal python - # # module name, e.g. if a directory in $PYTHONPATH has files named - # # "try.py" or "123.py", pkgutil will return entries named "try" or - # # "123". Filter those out. - # module_names = [m for m in module_names if is_identifier(m)] - # # Canonicalize. - # return tuple(ModuleHandle(m) for m in sorted(set(module_names))) + Also exclude any module names that are not legal python module names (e.g. + "try.py" or "123.py"). + :return: A list of all importable module names + """ with ExcludeImplicitCwdFromPathCtx(): - return [mod.name for mod in pkgutil.iter_modules() if is_identifier(mod.name)] + return [mod.name for mod in fast_iter_modules() if is_identifier(mod.name)] @cached_property def submodules(self): @@ -515,3 +501,96 @@ def containing(cls, identifier): module = cls(result) logger.debug("Imported %r to get %r", module, identifier) return module + + +def _fast_iter_finder_modules(importer: Any, prefix: str = '') -> Generator[tuple[str, bool], None, None]: + """Return an iterator over the modules for an importer. + + See pkgutil._iter_file_finder_modules for original implementation. Changes from + the original: + + - Use `os.scandir` instead of `os.listdir`, as the DirEntry objects returned + include the original path and the filename already, avoiding the need to + get this later + - Remove `inspect` import since it isn't being used + - Call out to `fast_getmodulename` rather than `inspect.getmodulename` for + speed + + :param prefix: A string prefix to append to the module names returned by this function + :param importer: Finder which targets a path to packages which can be imported + :return: The modules found by the importer, and whether they are packages or not + """ + if importer.path is None or not os.path.isdir(importer.path): + return + + yielded = {} + try: + direntries = sorted(os.scandir(importer.path), key=lambda de: de.name) + except OSError: + # ignore unreadable directories like import does + direntries = [] + + for entry in direntries: + modname = fast_getmodulename(entry.name) + if modname == '__init__' or modname in yielded: + continue + + path = os.path.join(importer.path, entry.path) + ispkg = False + + if not modname and os.path.isdir(path) and '.' not in entry.name: + modname = entry.name + try: + dircontents = os.scandir(path) + except OSError: + # ignore unreadable directories like import does + dircontents = [] # type: ignore[assignment] + for subentry in dircontents: + subname = fast_getmodulename(subentry.name) + if subname == '__init__': + ispkg = True + break + else: + continue # not a package + + if modname and '.' not in modname: + yielded[modname] = 1 + yield prefix + modname, ispkg + + +SUFFIXES = sorted((-len(suffix), suffix) for suffix in importlib.machinery.all_suffixes()) +def fast_getmodulename(fname: str) -> str | None: + """Get the module name for the given file path, or None. + + See `inspect.getmodulename` for original implementation. Changes from the original: + + - Importlib's suffixes have been generated and sorted at module initialization, rather + than on each call + + :param fname: Filename for which the module name is to be retrieved + :return: The module name, if this is a module; otherwise None + """ + for neglen, suffix in SUFFIXES: + if fname.endswith(suffix): + return fname[:neglen] + return None + + +def fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: + """Return an iterator over all importable python modules. + + This function patches `pkgutil.iter_importer_modules` for + `importlib.machinery.FileFinder` types, causing `pkgutil.iter_importer_modules` to + call our own custom _fast_iter_finder_modules instead of + pkgutil._iter_file_finder_modules. + + :return: The modules that are importable by python + """ + pkgutil.iter_importer_modules.register( # type: ignore[attr-defined] + importlib.machinery.FileFinder, _fast_iter_finder_modules + ) + yield from pkgutil.iter_modules() + pkgutil.iter_importer_modules.register( # type: ignore[attr-defined] + importlib.machinery.FileFinder, + pkgutil._iter_file_finder_modules, # type: ignore[attr-defined] + ) From 176b79033327d7d3d741f395c1c537c5c410f1ad Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 25 Jul 2025 15:03:52 -0700 Subject: [PATCH 04/41] Fix type annotation --- lib/python/pyflyby/_modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 5320bf98..c4dc1efa 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -2,7 +2,7 @@ # Copyright (C) 2011, 2012, 2013, 2014, 2015 Karl Chen. # License: MIT http://opensource.org/licenses/MIT -from __future__ import print_function +from __future__ import annotations import ast from functools import cached_property, total_ordering From 315f124f008ce855948ee7df75246f34e0665126 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Mon, 11 Aug 2025 15:06:10 -0700 Subject: [PATCH 05/41] cpp extension boilerplate --- meson.build | 53 ++++++++++++++++++++++++++++++++++++++ pyproject.toml | 7 +++-- src/_fast_iter_modules.cpp | 0 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 meson.build create mode 100644 src/_fast_iter_modules.cpp diff --git a/meson.build b/meson.build new file mode 100644 index 00000000..10ffd6b7 --- /dev/null +++ b/meson.build @@ -0,0 +1,53 @@ +project( + 'pyflyby', + 'cpp', + version: '0.0.1' +) + +py_mod = import('python') +py = py_mod.find_installation() + +pybind11_dep = dependency('pybind11', version: '>=2.10.4') + +includes = include_directories( + [ + 'src' + ] +) + +message('Installation directory: ', py.get_install_dir(subdir: 'pyflyby')) +message('Installation directory (not pure): ', py.get_install_dir(pure: false)) +message('Installation directory (pure): ', py.get_install_dir(pure: true)) + +# subdir('lib') + +install_data( + ['libexec/pyflyby/colordiff', 'libexec/pyflyby/diff-colorize'], + install_dir: py.get_install_dir( + subdir: 'libexec/pyflyby', + pure: true, # Not really pure, these are bash scripts, but we're not compiling them. + ) +) + +install_data( + [ + './etc/pyflyby/canonical.py', + './etc/pyflyby/common.py', + './etc/pyflyby/forget.py', + './etc/pyflyby/mandatory.py', + './etc/pyflyby/numpy.py', + './etc/pyflyby/std.py', + ], + install_dir: py.get_install_dir( + subdir: 'etc/pyflyby', + pure: true, + ) +) + +install_data( + ['./lib/emacs/pyflyby.el'], + install_dir: py.get_install_dir( + subdir: 'share/emacs/site-lisp', + pure: true, + ) +) diff --git a/pyproject.toml b/pyproject.toml index 6a68fee8..720259b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,9 @@ [build-system] -requires = ["setuptools >= 61.0"] -build-backend = "setuptools.build_meta" +requires = [ + "meson-python", + "pybind11>=2.10.4", +] +build-backend = "mesonpy" [tool.mypy] files = ['lib'] diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp new file mode 100644 index 00000000..e69de29b From 7e61b645bff037de1950292fbdac8e1872b392cd Mon Sep 17 00:00:00 2001 From: pdmurray Date: Mon, 11 Aug 2025 18:54:40 -0700 Subject: [PATCH 06/41] meson build is now working --- lib/python/pyflyby/meson.build | 34 +++++ meson.build | 52 +++++-- pyproject.toml | 63 +++++++++ setup.py | 239 --------------------------------- src/_fast_iter_modules.cpp | 22 +++ 5 files changed, 157 insertions(+), 253 deletions(-) create mode 100644 lib/python/pyflyby/meson.build delete mode 100755 setup.py diff --git a/lib/python/pyflyby/meson.build b/lib/python/pyflyby/meson.build new file mode 100644 index 00000000..0d428135 --- /dev/null +++ b/lib/python/pyflyby/meson.build @@ -0,0 +1,34 @@ +py.install_sources( + [ + '__init__.py', + '__main__.py', + '_autoimp.py', + '_cmdline.py', + '_comms.py', + '_dbg.py', + '_docxref.py', + '_dynimp.py', + '_file.py', + '_flags.py', + '_format.py', + '_idents.py', + '_import_sorting.py', + '_importclns.py', + '_importdb.py', + '_imports2s.py', + '_importstmt.py', + '_interactive.py', + '_livepatch.py', + '_log.py', + '_modules.py', + '_parse.py', + '_py.py', + '_saveframe.py', + '_saveframe_reader.py', + '_util.py', + '_version.py', + 'autoimport.py', + 'importdb.py', + ], + subdir: 'pyflyby', +) diff --git a/meson.build b/meson.build index 10ffd6b7..ec5b2985 100644 --- a/meson.build +++ b/meson.build @@ -1,25 +1,23 @@ project( 'pyflyby', 'cpp', - version: '0.0.1' + version: run_command( + [ + 'python', + '-c', + 'from pyflyby._version import __version__; print(__version__)' + ], + check: true, + env: {'PYTHONPATH': join_paths(meson.current_source_dir(), 'lib/python')} + ).stdout().strip() ) py_mod = import('python') -py = py_mod.find_installation() - +py = py_mod.find_installation(pure: false) pybind11_dep = dependency('pybind11', version: '>=2.10.4') +includes = include_directories([ 'src' ]) -includes = include_directories( - [ - 'src' - ] -) - -message('Installation directory: ', py.get_install_dir(subdir: 'pyflyby')) -message('Installation directory (not pure): ', py.get_install_dir(pure: false)) -message('Installation directory (pure): ', py.get_install_dir(pure: true)) - -# subdir('lib') +subdir('lib/python/pyflyby') install_data( ['libexec/pyflyby/colordiff', 'libexec/pyflyby/diff-colorize'], @@ -51,3 +49,29 @@ install_data( pure: true, ) ) + +install_data( + [ + 'bin/collect-exports', + 'bin/collect-imports', + 'bin/find-import', + 'bin/list-bad-xrefs', + 'bin/prune-broken-imports', + 'bin/pyflyby-diff', + 'bin/reformat-imports', + 'bin/replace-star-imports', + 'bin/saveframe', + 'bin/tidy-imports', + 'bin/transform-imports', + ], + install_dir: get_option('bindir') +) + +py.extension_module( + '_fast_iter_modules', + 'src/_fast_iter_modules.cpp', + install: true, + subdir: 'pyflyby', + dependencies: [pybind11_dep], + include_directories: includes +) diff --git a/pyproject.toml b/pyproject.toml index 720259b2..5e919930 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,6 +5,69 @@ requires = [ ] build-backend = "mesonpy" +[project] +name = "pyflyby" +dynamic = ["version"] +authors = [ + { name = "Karl Chen", email = "quarl@8166.clguba.z.quarl.org" }, +] +description = "pyflyby - Python development productivity tools, in particular automatic import management" +license = "MIT" +license-files = ["LICENSE.txt"] +readme = "README.rst" +requires-python = ">=3.9" +classifiers = [ + "Programming Language :: Python", + "Topic :: Software Development", + "Topic :: Software Development :: Code Generators", + "Topic :: Software Development :: Interpreters", + "Intended Audience :: Developers", + "Operating System :: OS Independent", +] +dependencies = [ + "six", + "toml", + "black", + "typing_extensions>=4.6; python_version<'3.12'" +] +[project.urls] +Homepage = "https://pypi.org/project/pyflyby/" +Documentation = "https://deshaw.github.io/pyflyby/" +Source = "https://github.com/deshaw/pyflyby" + +[project.scripts] +py = "pyflyby._py:py_main" +py3 = "pyflyby._py:py_main" + +[project.optional-dependencies] +test = [ + 'coverage', + 'epydoc', + 'flaky', + 'ipykernel>=5.4.3', + 'ipython', + 'jupyter', + 'jupyter_console>=6.2', + 'notebook<6.1', + 'pexpect>=3.3', + 'pytest-cov', + 'pytest-json-report', + 'pytest<=8', + 'requests', + 'rlipython', +] +lint = [ + 'flake8', + 'mypy', + 'pyflakes', + 'types-six' +] +docs = [ + 'sphinx', + 'sphinx_rtd_theme', + 'sphinx-autodoc-typehints', +] + [tool.mypy] files = ['lib'] #warn_incomplete_stub = false diff --git a/setup.py b/setup.py deleted file mode 100755 index ec8a7529..00000000 --- a/setup.py +++ /dev/null @@ -1,239 +0,0 @@ -#!/usr/bin/env python - -# pyflyby/setup.py. - -# License for THIS FILE ONLY: CC0 Public Domain Dedication -# http://creativecommons.org/publicdomain/zero/1.0/ - - - -import glob -import os -import re -from setuptools import Command, setup -from setuptools.command.test import test as TestCommand -from setuptools.command.sdist import sdist as SdistCommand -import subprocess -import sys -from textwrap import dedent - - -PYFLYBY_HOME = os.path.abspath(os.path.dirname(__file__)) -PYFLYBY_PYPATH = os.path.join(PYFLYBY_HOME, "lib/python") -PYFLYBY_DOT_PYFLYBY = os.path.join(PYFLYBY_HOME, ".pyflyby") - -# Get the pyflyby version from pyflyby.__version__. -# We use exec instead to avoid importing pyflyby here. -version_vars = {} -version_fn = os.path.join(PYFLYBY_PYPATH, "pyflyby/_version.py") -exec(open(version_fn).read(), {}, version_vars) -version = version_vars["__version__"] - - -def read(fname): - with open(os.path.join(PYFLYBY_HOME, fname)) as f: - return f.read() - - -def list_python_source_files(): - results = [] - for fn in glob.glob("bin/*"): - if not os.path.isfile(fn): - continue - with open(fn) as f: - line = f.readline() - if not re.match("^#!.*python", line): - continue - results.append(fn) - results += glob.glob("lib/python/pyflyby/*.py") - results += glob.glob("tests/*.py") - return results - - -class TidyImports(Command): - description = "tidy imports in pyflyby source files (for maintainer use)" - - user_options = [] - - def initialize_options(self): - pass - - def finalize_options(self): - pass - - def run(self): - files = list_python_source_files() - pyflyby_path = ":".join([ - os.path.join(PYFLYBY_HOME, "etc/pyflyby"), - PYFLYBY_DOT_PYFLYBY, - ]) - subprocess.call([ - "env", - "PYFLYBY_PATH=%s" % (pyflyby_path,), - "tidy-imports", - # "--debug", - "--uniform", - ] + files) - - -class CollectImports(Command): - description = "update pyflyby's own .pyflyby file from imports (for maintainer use)" - - user_options = [] - - def initialize_options(self): - pass - - def finalize_options(self): - pass - - def run(self): - files = list_python_source_files() - print("Rewriting", PYFLYBY_DOT_PYFLYBY) - with open(PYFLYBY_DOT_PYFLYBY, 'w') as f: - print(dedent(""" - # -*- python -*- - # - # This is the imports database file for pyflyby itself. - # - # To regenerate this file, run: setup.py collect_imports - - __mandatory_imports__ = [ - 'from __future__ import print_function', - ] - """).lstrip(), file=f) - f.flush() - subprocess.call( - [ - os.path.join(PYFLYBY_HOME, "bin/collect-imports"), - "--include=pyflyby", - "--uniform", - ] + files, - stdout=f) - subprocess.call(["git", "diff", PYFLYBY_DOT_PYFLYBY]) - - -class PyTest(TestCommand): - user_options = [('pytest-args=', 'a', "Arguments to pass to py.test")] - - def initialize_options(self): - TestCommand.initialize_options(self) - self.pytest_args = ['--doctest-modules', 'lib', 'tests'] - - def finalize_options(self): - TestCommand.finalize_options(self) - self.test_args = [] - self.test_suite = True - - def run_tests(self): - import pytest - # We want to test the version of pyflyby in this repository. It's - # possible that some different version of pyflyby already got imported - # in usercustomize, before we could set sys.path here. If so, unload - # it. - if 'pyflyby' in sys.modules: - print("setup.py: Unloading %s from sys.modules " - "(perhaps it got loaded in usercustomize?)" - % (sys.modules['pyflyby'].__file__,)) - del sys.modules['pyflyby'] - for k in sys.modules.keys(): - if k.startswith("pyflyby."): - del sys.modules[k] - # Add our version of pyflyby to sys.path & PYTHONPATH. - sys.path.insert(0, PYFLYBY_PYPATH) - os.environ["PYTHONPATH"] = PYFLYBY_PYPATH - # Run pytest. - errno = pytest.main(self.pytest_args) - sys.exit(errno) - - -DISALLOWED_CONTENT = ['g'+'uas', 'g'+'ql'] - -def check_for_disallowed_content(archive_filename): - archive_filename = os.path.abspath(archive_filename) - assert archive_filename.endswith(".tar.gz") - archive_members = subprocess.check_output(['tar', 'tzf', archive_filename]) - archive_file_content = subprocess.check_output(['tar', 'xzOf', archive_filename]) - data = archive_members + archive_file_content.lower() - for disallowed in DISALLOWED_CONTENT: - if disallowed.encode("ascii") in data: - raise ValueError("Found match for content that shouldn't be source-disted: %s" - % (disallowed,)) - - -class SdistAndCheck(SdistCommand, object): - - def make_distribution(self): - super(SdistAndCheck, self).make_distribution() - for filename in self.archive_files: - check_for_disallowed_content(filename) - - -setup( - name = "pyflyby", - version = version, - author = "Karl Chen", - author_email = "quarl@8166.clguba.z.quarl.org", - description = ("pyflyby - Python development productivity tools, in particular automatic import management"), - license = "MIT", - keywords = "pyflyby py autopython autoipython productivity automatic imports autoimporter tidy-imports", - url = "https://pypi.org/project/pyflyby/", - project_urls={ - 'Documentation': 'https://deshaw.github.io/pyflyby/', - 'Source' : 'https://github.com/deshaw/pyflyby', - }, - package_dir={'': 'lib/python'}, - packages=['pyflyby'], - entry_points={'console_scripts': - '\n'.join([ - 'py=pyflyby._py:py_main', - 'py3=pyflyby._py:py_main', - ])}, - scripts=[ - # TODO: convert these scripts into entry points (but leave stubs in - # bin/ for non-installed usage) - 'bin/collect-exports', - 'bin/collect-imports', - 'bin/find-import', - 'bin/list-bad-xrefs', - 'bin/prune-broken-imports', - 'bin/pyflyby-diff', - 'bin/reformat-imports', - 'bin/replace-star-imports', - 'bin/saveframe', - 'bin/tidy-imports', - 'bin/transform-imports', - ], - data_files=[ - ('libexec/pyflyby', [ - 'libexec/pyflyby/colordiff', 'libexec/pyflyby/diff-colorize', - ]), - ('etc/pyflyby', glob.glob('etc/pyflyby/*.py')), - ('share/doc/pyflyby', glob.glob('doc/*.txt')), - ('share/emacs/site-lisp', ['lib/emacs/pyflyby.el']), - ], - long_description=read('README.rst'), - classifiers=[ - "Development Status :: 5 - Production/Stable", - "Topic :: Software Development", - "Topic :: Software Development :: Code Generators", - "Topic :: Software Development :: Interpreters", - "Intended Audience :: Developers", - "License :: OSI Approved :: MIT License", - "Programming Language :: Python", - ], - install_requires=[ - "six", - "toml", - "black", - "typing_extensions>=4.6; python_version<'3.12'" - ], - python_requires=">3.9, <4", - tests_require=['pexpect>=3.3', 'pytest', 'epydoc', 'rlipython', 'requests'], - cmdclass = { - 'test' : PyTest, - 'sdist' : SdistAndCheck, - 'collect_imports': CollectImports, - 'tidy_imports' : TidyImports, - }, -) diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp index e69de29b..a24d8d82 100644 --- a/src/_fast_iter_modules.cpp +++ b/src/_fast_iter_modules.cpp @@ -0,0 +1,22 @@ +#include +#include + +namespace py = pybind11; + +/** + * @brief Get the list of python modules. + * + */ +py::tuple iter_modules() { + return py::make_tuple(); +} + +PYBIND11_MODULE(_iter_modules, m) { + m.doc() = "A fast version of pkgutil.iter_modules()."; + m.def( + "iter_modules", + &iter_modules, + "A fast implementation of pkgutil.iter_modules(path=None, prefix='')", + py::return_value_policy::take_ownership + ); +} From f33d3e18008b7f7dfa46e1b515a07bebe684aa8d Mon Sep 17 00:00:00 2001 From: pdmurray Date: Tue, 12 Aug 2025 10:39:33 -0700 Subject: [PATCH 07/41] WIP --- lib/python/pyflyby/_modules.py | 29 +++++++++++++++------------ src/_fast_iter_modules.cpp | 36 ++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index c4dc1efa..6f848ead 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -539,19 +539,24 @@ def _fast_iter_finder_modules(importer: Any, prefix: str = '') -> Generator[tupl ispkg = False if not modname and os.path.isdir(path) and '.' not in entry.name: - modname = entry.name - try: - dircontents = os.scandir(path) - except OSError: - # ignore unreadable directories like import does - dircontents = [] # type: ignore[assignment] - for subentry in dircontents: - subname = fast_getmodulename(subentry.name) - if subname == '__init__': - ispkg = True - break + if os.path.join(path, '__init__.py'): + ispkg = True else: - continue # not a package + continue # not a package + + # modname = entry.name + # try: + # dircontents = os.scandir(path) + # except OSError: + # # ignore unreadable directories like import does + # dircontents = [] # type: ignore[assignment] + # for subentry in dircontents: + # subname = fast_getmodulename(subentry.name) + # if subname == '__init__': + # ispkg = True + # break + # else: + # continue # not a package if modname and '.' not in modname: yielded[modname] = 1 diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp index a24d8d82..1b11f244 100644 --- a/src/_fast_iter_modules.cpp +++ b/src/_fast_iter_modules.cpp @@ -1,22 +1,46 @@ +#include "pybind11/cast.h" +#include #include +#include #include +#include +#include namespace py = pybind11; +namespace fs = std::filesystem; /** * @brief Get the list of python modules. * */ -py::tuple iter_modules() { +std::optional> _iter_file_finder_modules(py::object importer, std::string prefix) { + + std::vector ret; + + py::object path_obj = importer.attr("path"); + if (path_obj.is_none()) { + return ret; + } + + auto path = fs::path(py::str(path_obj).cast()); + if (!fs::is_directory(path)) { + return ret; + } + + // bool filesystem + + py::object importer_path = importer.attr("path"); return py::make_tuple(); } -PYBIND11_MODULE(_iter_modules, m) { - m.doc() = "A fast version of pkgutil.iter_modules()."; +PYBIND11_MODULE(_fast_iter_modules, m) { + m.doc() = "A fast version of pkgutil._iter_file_finder_modules."; m.def( - "iter_modules", - &iter_modules, - "A fast implementation of pkgutil.iter_modules(path=None, prefix='')", + "_iter_file_finder_modules", + &_iter_file_finder_modules, + "A fast implementation of pkgutil._iter_file_finder_modules(importer, prefix='')", + py::arg("importer"), + py::arg("prefix") = py::str(""), py::return_value_policy::take_ownership ); } From 2bba1556fbbc33b38cdb46864852061d31266f80 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Tue, 12 Aug 2025 12:48:31 -0700 Subject: [PATCH 08/41] wip --- src/_fast_iter_modules.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp index 1b11f244..16a6c637 100644 --- a/src/_fast_iter_modules.cpp +++ b/src/_fast_iter_modules.cpp @@ -13,7 +13,7 @@ namespace fs = std::filesystem; * @brief Get the list of python modules. * */ -std::optional> _iter_file_finder_modules(py::object importer, std::string prefix) { +std::vector _iter_file_finder_modules(py::object importer, std::string prefix) { std::vector ret; @@ -30,7 +30,7 @@ std::optional> _iter_file_finder_modules(py::object imp // bool filesystem py::object importer_path = importer.attr("path"); - return py::make_tuple(); + return ret; } PYBIND11_MODULE(_fast_iter_modules, m) { From 85db618437be5471ed6f4562dadb7b5f8b6163ff Mon Sep 17 00:00:00 2001 From: pdmurray Date: Tue, 12 Aug 2025 22:04:15 -0700 Subject: [PATCH 09/41] Implement a compiled extension which finds modules --- lib/python/pyflyby/_modules.py | 85 ++-------------------------------- meson.build | 10 +--- src/_fast_iter_modules.cpp | 74 +++++++++++++++++++++++++---- 3 files changed, 71 insertions(+), 98 deletions(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 6f848ead..8404bde6 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -5,7 +5,7 @@ from __future__ import annotations import ast -from functools import cached_property, total_ordering +from functools import cached_property, total_ordering, partial import importlib import itertools import os @@ -16,6 +16,7 @@ from pyflyby._log import logger from pyflyby._util import (ExcludeImplicitCwdFromPathCtx, cmp, memoize, prefixes) +from ._fast_iter_modules import _iter_file_finder_modules import re from six import reraise @@ -502,85 +503,7 @@ def containing(cls, identifier): logger.debug("Imported %r to get %r", module, identifier) return module - -def _fast_iter_finder_modules(importer: Any, prefix: str = '') -> Generator[tuple[str, bool], None, None]: - """Return an iterator over the modules for an importer. - - See pkgutil._iter_file_finder_modules for original implementation. Changes from - the original: - - - Use `os.scandir` instead of `os.listdir`, as the DirEntry objects returned - include the original path and the filename already, avoiding the need to - get this later - - Remove `inspect` import since it isn't being used - - Call out to `fast_getmodulename` rather than `inspect.getmodulename` for - speed - - :param prefix: A string prefix to append to the module names returned by this function - :param importer: Finder which targets a path to packages which can be imported - :return: The modules found by the importer, and whether they are packages or not - """ - if importer.path is None or not os.path.isdir(importer.path): - return - - yielded = {} - try: - direntries = sorted(os.scandir(importer.path), key=lambda de: de.name) - except OSError: - # ignore unreadable directories like import does - direntries = [] - - for entry in direntries: - modname = fast_getmodulename(entry.name) - if modname == '__init__' or modname in yielded: - continue - - path = os.path.join(importer.path, entry.path) - ispkg = False - - if not modname and os.path.isdir(path) and '.' not in entry.name: - if os.path.join(path, '__init__.py'): - ispkg = True - else: - continue # not a package - - # modname = entry.name - # try: - # dircontents = os.scandir(path) - # except OSError: - # # ignore unreadable directories like import does - # dircontents = [] # type: ignore[assignment] - # for subentry in dircontents: - # subname = fast_getmodulename(subentry.name) - # if subname == '__init__': - # ispkg = True - # break - # else: - # continue # not a package - - if modname and '.' not in modname: - yielded[modname] = 1 - yield prefix + modname, ispkg - - -SUFFIXES = sorted((-len(suffix), suffix) for suffix in importlib.machinery.all_suffixes()) -def fast_getmodulename(fname: str) -> str | None: - """Get the module name for the given file path, or None. - - See `inspect.getmodulename` for original implementation. Changes from the original: - - - Importlib's suffixes have been generated and sorted at module initialization, rather - than on each call - - :param fname: Filename for which the module name is to be retrieved - :return: The module name, if this is a module; otherwise None - """ - for neglen, suffix in SUFFIXES: - if fname.endswith(suffix): - return fname[:neglen] - return None - - +SUFFIXES = sorted(importlib.machinery.all_suffixes()) def fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: """Return an iterator over all importable python modules. @@ -592,7 +515,7 @@ def fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: :return: The modules that are importable by python """ pkgutil.iter_importer_modules.register( # type: ignore[attr-defined] - importlib.machinery.FileFinder, _fast_iter_finder_modules + importlib.machinery.FileFinder, partial(_iter_file_finder_modules, suffixes=SUFFIXES) ) yield from pkgutil.iter_modules() pkgutil.iter_importer_modules.register( # type: ignore[attr-defined] diff --git a/meson.build b/meson.build index ec5b2985..79cac4f7 100644 --- a/meson.build +++ b/meson.build @@ -1,15 +1,7 @@ project( 'pyflyby', 'cpp', - version: run_command( - [ - 'python', - '-c', - 'from pyflyby._version import __version__; print(__version__)' - ], - check: true, - env: {'PYTHONPATH': join_paths(meson.current_source_dir(), 'lib/python')} - ).stdout().strip() + version: '1.9.13', ) py_mod = import('python') diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp index 16a6c637..f3865ae8 100644 --- a/src/_fast_iter_modules.cpp +++ b/src/_fast_iter_modules.cpp @@ -1,35 +1,92 @@ #include "pybind11/cast.h" -#include +#include "pybind11/pytypes.h" #include #include #include #include +#include #include namespace py = pybind11; namespace fs = std::filesystem; + +/** + * @brief Fast equivalent of `inspect.getmodulename`. + * + * @param path Path to a file + * @param suffixes Suffixes of valid python modules. Typically this is + * `importlib.machinery.all_suffixes()` + * @return The stem of the file, if this is a module; empty string otherwise + */ +std::string getmodulename(fs::path path, std::vector suffixes) { + fs::path ext = path.extension(); + for (auto const& suffix : suffixes) { + if (ext == suffix) { + return path.stem(); + } + } + return ""; +} + /** - * @brief Get the list of python modules. + * @brief Get a list of importable python modules. + * + * See `pkgutil._iter_file_finder_modules` for the original python version. * + * @param importer Importer instance containing an import path. Typically this is an object of type + * `importlib.machinery.FileFinder` + * @param prefix A string prefix to affix to the front of all returned modules + * @param suffixes Suffixes of valid python modules. Typically this is + * `importlib.machinery.all_suffixes()` + * @return A vector of tuples containing modules names, and a boolean indicating whether the module + * is a package or not */ -std::vector _iter_file_finder_modules(py::object importer, std::string prefix) { +std::vector> _iter_file_finder_modules( + py::object importer, std::string prefix, std::vector suffixes +) { - std::vector ret; + std::vector> ret; + // The importer doesn't have a path py::object path_obj = importer.attr("path"); if (path_obj.is_none()) { return ret; } - auto path = fs::path(py::str(path_obj).cast()); - if (!fs::is_directory(path)) { + // The importer's path isn't an existing directory + fs::path path = fs::path(py::str(path_obj).cast()); + if (!fs::is_directory(path) || !fs::exists(path)) { return ret; } - // bool filesystem + for (auto const& entry : fs::directory_iterator(path)) { + fs::path entry_path = entry.path(); + std::string modname = getmodulename(entry_path, suffixes); + + if ( + modname == "" + && fs::is_directory(entry_path) + && entry_path.string().find(".") == std::string::npos + ) { + ret.push_back( + std::make_tuple( + prefix + entry_path.string(), + fs::is_regular_file(entry_path / "__init__.py") // Is this a package? + ) + ); + } else if (modname == "__init__") { + continue; + } else if (modname != "" && modname.find(".") == std::string::npos){ + ret.push_back( + std::make_tuple( + prefix + modname, + false // This is definitely not a package + ) + ); + } + } - py::object importer_path = importer.attr("path"); return ret; } @@ -41,6 +98,7 @@ PYBIND11_MODULE(_fast_iter_modules, m) { "A fast implementation of pkgutil._iter_file_finder_modules(importer, prefix='')", py::arg("importer"), py::arg("prefix") = py::str(""), + py::arg("suffixes") = std::make_tuple(".py", ".pyc"), py::return_value_policy::take_ownership ); } From f8110b3935b71a3cb5a9996822affe29cddd8132 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Tue, 12 Aug 2025 23:25:22 -0700 Subject: [PATCH 10/41] Compiled extension tested, working --- lib/python/pyflyby/_modules.py | 2 +- src/_fast_iter_modules.cpp | 20 +++++++++----------- tests/test_modules.py | 10 +++++++++- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 8404bde6..f0c5e116 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -16,7 +16,7 @@ from pyflyby._log import logger from pyflyby._util import (ExcludeImplicitCwdFromPathCtx, cmp, memoize, prefixes) -from ._fast_iter_modules import _iter_file_finder_modules +from pyflyby._fast_iter_modules import _iter_file_finder_modules import re from six import reraise diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp index f3865ae8..49ee2cfc 100644 --- a/src/_fast_iter_modules.cpp +++ b/src/_fast_iter_modules.cpp @@ -22,8 +22,10 @@ namespace fs = std::filesystem; std::string getmodulename(fs::path path, std::vector suffixes) { fs::path ext = path.extension(); for (auto const& suffix : suffixes) { - if (ext == suffix) { - return path.stem(); + std::string path_str = path.string(); + std::string::size_type pos = path_str.rfind(suffix); + if (pos != std::string::npos) { + return path_str.substr(0, pos); } } return ""; @@ -45,7 +47,6 @@ std::string getmodulename(fs::path path, std::vector suffixes) { std::vector> _iter_file_finder_modules( py::object importer, std::string prefix, std::vector suffixes ) { - std::vector> ret; // The importer doesn't have a path @@ -62,19 +63,16 @@ std::vector> _iter_file_finder_modules( for (auto const& entry : fs::directory_iterator(path)) { fs::path entry_path = entry.path(); - std::string modname = getmodulename(entry_path, suffixes); + fs::path filename = entry_path.filename(); + std::string modname = getmodulename(filename, suffixes); if ( modname == "" && fs::is_directory(entry_path) - && entry_path.string().find(".") == std::string::npos + && filename.string().find(".") == std::string::npos + && fs::is_regular_file(entry_path / "__init__.py") // Is this a package? ) { - ret.push_back( - std::make_tuple( - prefix + entry_path.string(), - fs::is_regular_file(entry_path / "__init__.py") // Is this a package? - ) - ); + ret.push_back(std::make_tuple(prefix + filename.string(), true)); } else if (modname == "__init__") { continue; } else if (modname != "" && modname.find(".") == std::string::npos){ diff --git a/tests/test_modules.py b/tests/test_modules.py index 1a111fbc..0893c6e5 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -9,7 +9,8 @@ import logging.handlers from pyflyby._file import Filename from pyflyby._idents import DottedIdentifier -from pyflyby._modules import ModuleHandle +from pyflyby._modules import ModuleHandle, fast_iter_modules +from pkgutil import iter_modules import re import subprocess import sys @@ -110,3 +111,10 @@ def test_filename_noload_1(modname): assert ret.returncode != 121, f"{modname} imported by pyflyby import" assert ret.returncode != 120, f"{modname} in sys.modules at startup" assert ret.returncode == 0, (ret, ret.stdout, ret.stderr) + +def test_fast_iter_modules(): + """Test that the cpp extension finds the same modules as pkgutil.iter_modules.""" + fast = sorted(list(fast_iter_modules()), key=lambda x: x.name) + slow = sorted(list(iter_modules()), key=lambda x: x.name) + + assert fast == slow From b373664aaac631fc025840d8434e329a36d9e399 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Tue, 12 Aug 2025 23:28:48 -0700 Subject: [PATCH 11/41] Don't use `python setup.py ` --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1b1802b0..73828dbc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,12 +31,12 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install and update Python dependencies on Python 3 run: | - python -m pip install --upgrade pip setuptools wheel + python -m pip install --upgrade pip setuptools wheel build python -m pip install --upgrade "pexpect>=3.3" 'pytest<=8' rlipython 'ipykernel>=5.4.3' requests jupyter flaky 'notebook<6.1' wheel 'jupyter_console>=6.2' pytest-cov ipython coverage pytest-json-report pip install -e . - name: test release build run: | - python setup.py sdist bdist_wheel + python -m build - name: compileall run: | python -We:invalid -m compileall -f -q lib/ etc/; From 922620d7724c329af3e7915f8b72d704afc12ce2 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 00:06:48 -0700 Subject: [PATCH 12/41] Don't use editable installs for testing --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 73828dbc..5a517981 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,7 +33,7 @@ jobs: run: | python -m pip install --upgrade pip setuptools wheel build python -m pip install --upgrade "pexpect>=3.3" 'pytest<=8' rlipython 'ipykernel>=5.4.3' requests jupyter flaky 'notebook<6.1' wheel 'jupyter_console>=6.2' pytest-cov ipython coverage pytest-json-report - pip install -e . + pip install . - name: test release build run: | python -m build From a63ebe13bb1ee2d8a21b4726c0a0f3f6bd0336f1 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 00:08:26 -0700 Subject: [PATCH 13/41] Don't build/test with build isolation --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5a517981..625ffe76 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,9 +31,9 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install and update Python dependencies on Python 3 run: | - python -m pip install --upgrade pip setuptools wheel build + python -m pip install --upgrade pip setuptools wheel build meson-python python -m pip install --upgrade "pexpect>=3.3" 'pytest<=8' rlipython 'ipykernel>=5.4.3' requests jupyter flaky 'notebook<6.1' wheel 'jupyter_console>=6.2' pytest-cov ipython coverage pytest-json-report - pip install . + pip install -ve --no-build-isolation . - name: test release build run: | python -m build From ecabfa2de3cfc50c329ee3217f2e1c1ec0a9e293 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 07:55:55 -0700 Subject: [PATCH 14/41] Switch order of args --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 625ffe76..f6468a4d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,7 +33,7 @@ jobs: run: | python -m pip install --upgrade pip setuptools wheel build meson-python python -m pip install --upgrade "pexpect>=3.3" 'pytest<=8' rlipython 'ipykernel>=5.4.3' requests jupyter flaky 'notebook<6.1' wheel 'jupyter_console>=6.2' pytest-cov ipython coverage pytest-json-report - pip install -ve --no-build-isolation . + pip install --no-build-isolation -ve . - name: test release build run: | python -m build From 025bcdca48911327ecac360b3e758f4a396be13a Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 09:26:00 -0700 Subject: [PATCH 15/41] Correct a bad docstring --- lib/python/pyflyby/_modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index f0c5e116..0887c9f7 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -509,7 +509,7 @@ def fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: This function patches `pkgutil.iter_importer_modules` for `importlib.machinery.FileFinder` types, causing `pkgutil.iter_importer_modules` to - call our own custom _fast_iter_finder_modules instead of + call our own custom _iter_file_finder_modules instead of pkgutil._iter_file_finder_modules. :return: The modules that are importable by python From 8aa4381a20864b45e39ef6eccad90c446d7184f3 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 09:30:32 -0700 Subject: [PATCH 16/41] Add pybind11 dep to CI... --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9eb106c6..91373cfc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,7 +31,7 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install and update Python dependencies on Python 3 run: | - python -m pip install --upgrade pip setuptools wheel build meson-python + python -m pip install --upgrade pip setuptools wheel build meson-python pybind11 python -m pip install --upgrade "pexpect>=3.3" 'pytest<=8' rlipython 'ipykernel>=5.4.3' requests jupyter flaky 'notebook<6.1' wheel 'jupyter_console>=6.2' pytest-cov ipython coverage pytest-json-report hypothesis pip install --no-build-isolation -ve . - name: test release build From d9cd381fff11dd0607de29c55525ae08465e9fb7 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 10:06:54 -0700 Subject: [PATCH 17/41] Enforce c++17 standard --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index 79cac4f7..fa788e9f 100644 --- a/meson.build +++ b/meson.build @@ -2,6 +2,7 @@ project( 'pyflyby', 'cpp', version: '1.9.13', + default_options: 'cpp_std=c++17' ) py_mod = import('python') From 0bbb60c519cd6a6465c1a61ae168829efe1ce4e7 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 10:32:34 -0700 Subject: [PATCH 18/41] Fix the doc build configuration --- .github/workflows/docs.yml | 3 --- doc/conf.py | 22 ++++++++++++++++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index f079400b..55c380a1 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -13,9 +13,6 @@ jobs: - name: Install dependencies run: | pip install sphinx sphinx_rtd_theme sphinx-autodoc-typehints - python -m pip install --upgrade pip setuptools wheel - python -m pip install --upgrade rlipython ipykernel==5.4.3 requests jupyter flaky 'notebook<6.1' 'prompt_toolkit<3.0.15' wheel 'jupyter_console>=6.2' 'pytest-cov<3' ipython 'coverage<6.3' pytest-json-report - pip install -e . - name: Build docs run: | make html diff --git a/doc/conf.py b/doc/conf.py index 835f38da..73c739ca 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -2,6 +2,8 @@ # -- Path setup -------------------------------------------------------------- import os +import pathlib +import re import sys sys.path.insert(0, os.path.abspath('../lib/python')) sys.path.insert(0, os.path.abspath('..')) @@ -11,9 +13,21 @@ copyright = '2019, Karl Chen' author = 'Karl Chen' # The full version, including alpha/beta/rc tags -import pyflyby -release = pyflyby.__version__ +def find_version(): + # Extract version information via regex to avoid importing + project_root = pathlib.Path(__file__).parent.parent + with open(project_root / "lib" / "python" / "pyflyby" / "_version.py") as f: + version_match = re.search( + r"^__version__ = ['\"](?P.*)['\"]$", + f.read(), + re.M, + ) + if version_match: + return version_match.group("version") + raise RuntimeError("Unable to find version string.") + +release = find_version() # -- General configuration --------------------------------------------------- @@ -29,6 +43,10 @@ 'private-members': True } +autodoc_mock_imports = [ + "pyflyby._fast_iter_modules" +] + html_theme_options = { 'collapse_navigation': False, 'navigation_depth': -1, From 9f8ef45fcee68c27992261d89cbf18198f73bfd8 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 10:57:25 -0700 Subject: [PATCH 19/41] Use dependency groups for docs and lint; don't build docs in test --- .github/workflows/docs.yml | 3 ++- .github/workflows/lint.yml | 20 ++++++-------------- .github/workflows/test.yml | 13 ++----------- pyproject.toml | 2 ++ 4 files changed, 12 insertions(+), 26 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 55c380a1..776a3e4a 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -12,7 +12,8 @@ jobs: - uses: actions/setup-python@v5 - name: Install dependencies run: | - pip install sphinx sphinx_rtd_theme sphinx-autodoc-typehints + pip install -U pip + pip install -v --group docs - name: Build docs run: | make html diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b3d032ba..ef4a8709 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -3,25 +3,18 @@ name: Lint on: [push, pull_request] jobs: - test: + lint: runs-on: "ubuntu-latest" - strategy: - fail-fast: false - matrix: - python-version: ["3.13"] - steps: - uses: actions/checkout@v4 - - name: Set up Python ${{ matrix.python-version }} + - name: Set up Python uses: actions/setup-python@v5 with: - python-version: ${{ matrix.python-version }} - - name: Install and update Python dependencies on Python 3 + python-version: 3.13 + - name: Install pyflyby run: | - python -m pip install --upgrade pip setuptools wheel - python -m pip install --upgrade pyflakes flake8 mypy - python -m pip install types-six - pip install -e . + pip install -U pip + pip install -v . --group lint - name: Mypy run: | mypy lib/python --ignore-missing-imports @@ -32,4 +25,3 @@ jobs: - name: Self-tidy-import run: | ./bin/tidy-imports -d lib/python/ tests/ - diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 91373cfc..9aa92fe5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,11 +29,9 @@ jobs: uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - - name: Install and update Python dependencies on Python 3 + - name: Install pyflyby run: | - python -m pip install --upgrade pip setuptools wheel build meson-python pybind11 - python -m pip install --upgrade "pexpect>=3.3" 'pytest<=8' rlipython 'ipykernel>=5.4.3' requests jupyter flaky 'notebook<6.1' wheel 'jupyter_console>=6.2' pytest-cov ipython coverage pytest-json-report hypothesis - pip install --no-build-isolation -ve . + pip install -v .[test] - name: test release build run: | python -m build @@ -61,10 +59,3 @@ jobs: name: pytest-timing-${{ matrix.os }}-${{ matrix.python-version }} path: ./report-*.json - uses: codecov/codecov-action@v5 - - name: Build docs - if: ${{ matrix.python-version == '3.11'}} - run: | - pip install sphinx sphinx_rtd_theme sphinx-autodoc-typehints - cd doc - make html - cd .. diff --git a/pyproject.toml b/pyproject.toml index 5e919930..e23a0bca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,6 +56,8 @@ test = [ 'requests', 'rlipython', ] + +[dependency-groups] lint = [ 'flake8', 'mypy', From f576055c67ea7c9e81704a57dd670c765d87d73f Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 11:58:48 -0700 Subject: [PATCH 20/41] Add 'build' as test dependency --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index e23a0bca..79a619b4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,6 +41,7 @@ py3 = "pyflyby._py:py_main" [project.optional-dependencies] test = [ + 'build', 'coverage', 'epydoc', 'flaky', From 445346f6d5e8c107c5fe00b22d8db94693ba7a02 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 22:01:13 -0700 Subject: [PATCH 21/41] Make tests run in editable mode --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9aa92fe5..f6df64b4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,7 +31,7 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install pyflyby run: | - pip install -v .[test] + pip install -ve .[test] - name: test release build run: | python -m build From 72bc1b4231577ce57ccd477c5aaf82bee120c527 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 22:32:58 -0700 Subject: [PATCH 22/41] Add hypothesis; follow meson-python docs around editable mode --- .github/workflows/lint.yml | 7 +++++-- .github/workflows/test.yml | 6 +++++- pyproject.toml | 1 + 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index ef4a8709..dd0aee5b 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -13,8 +13,11 @@ jobs: python-version: 3.13 - name: Install pyflyby run: | - pip install -U pip - pip install -v . --group lint + # Include build dependencies for run time; see + # https://mesonbuild.com/meson-python/how-to-guides/editable-installs.html#build-dependencies + # for details. + pip install meson-python meson ninja pybind11>=2.10.4 + pip install --no-build-isolation -ve . --group lint - name: Mypy run: | mypy lib/python --ignore-missing-imports diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f6df64b4..9844ad91 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,7 +31,11 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install pyflyby run: | - pip install -ve .[test] + # Include build dependencies for run time; see + # https://mesonbuild.com/meson-python/how-to-guides/editable-installs.html#build-dependencies + # for details. + pip install meson-python meson ninja pybind11>=2.10.4 + pip install --no-build-isolation -ve .[test] - name: test release build run: | python -m build diff --git a/pyproject.toml b/pyproject.toml index 79a619b4..db61d08d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,7 @@ test = [ 'coverage', 'epydoc', 'flaky', + 'hypothesis', 'ipykernel>=5.4.3', 'ipython', 'jupyter', From d5e5f7b96afe57185b1c21da799654b74a81d424 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 22:44:29 -0700 Subject: [PATCH 23/41] Make epydoc and wheel direct dependencies of pyflyby --- pyproject.toml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index db61d08d..8b8f08f9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,10 +25,12 @@ classifiers = [ "Operating System :: OS Independent", ] dependencies = [ + "black", "six", "toml", - "black", - "typing_extensions>=4.6; python_version<'3.12'" + "typing_extensions>=4.6; python_version<'3.12'", + 'epydoc', + 'wheel', # required by epydoc, but not listed as a dependency ] [project.urls] Homepage = "https://pypi.org/project/pyflyby/" @@ -43,7 +45,6 @@ py3 = "pyflyby._py:py_main" test = [ 'build', 'coverage', - 'epydoc', 'flaky', 'hypothesis', 'ipykernel>=5.4.3', @@ -64,7 +65,7 @@ lint = [ 'flake8', 'mypy', 'pyflakes', - 'types-six' + 'types-six', ] docs = [ 'sphinx', From 18a91010cd13822b28c1cb11fa8cd173547eee8a Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 13 Aug 2025 23:34:58 -0700 Subject: [PATCH 24/41] Add setuptools and wheel, required by epydoc --- .github/workflows/lint.yml | 1 + .github/workflows/test.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index dd0aee5b..5c2b71b9 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -17,6 +17,7 @@ jobs: # https://mesonbuild.com/meson-python/how-to-guides/editable-installs.html#build-dependencies # for details. pip install meson-python meson ninja pybind11>=2.10.4 + pip install setuptools wheel # needed for epydoc pip install --no-build-isolation -ve . --group lint - name: Mypy run: | diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9844ad91..511cee57 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -35,6 +35,7 @@ jobs: # https://mesonbuild.com/meson-python/how-to-guides/editable-installs.html#build-dependencies # for details. pip install meson-python meson ninja pybind11>=2.10.4 + pip install setuptools wheel # needed for epydoc pip install --no-build-isolation -ve .[test] - name: test release build run: | From 25eb12221e736111b7054ca524eca5bd32501fe2 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Thu, 14 Aug 2025 15:10:32 -0700 Subject: [PATCH 25/41] Make the install `pure: false`; prevent site-packages pollution --- README.rst | 4 +--- lib/python/pyflyby/_importdb.py | 2 +- lib/python/pyflyby/_interactive.py | 2 +- meson.build | 9 +++------ 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/README.rst b/README.rst index 592723cc..b7916481 100644 --- a/README.rst +++ b/README.rst @@ -459,7 +459,7 @@ Emacs support * To get a ``M-x tidy-imports`` command in GNU Emacs, add to your ``~/.emacs``:: - (load "/path/to/pyflyby/lib/emacs/pyflyby.el") + (load "//pyflyby/share/emacs/site-lisp/pyflyby.el") - Pyflyby.el doesn't yet work with XEmacs; patches welcome. @@ -546,5 +546,3 @@ Release 8. Check/update https://github.com/conda-forge/pyflyby-feedstock for new pyflyby release on conda-forge - - diff --git a/lib/python/pyflyby/_importdb.py b/lib/python/pyflyby/_importdb.py index fd69bc9c..54f44893 100644 --- a/lib/python/pyflyby/_importdb.py +++ b/lib/python/pyflyby/_importdb.py @@ -219,7 +219,7 @@ def __new__(cls, *args): return cls._from_data(arg, [], [], []) return cls._from_args(arg) # PythonBlock, Filename, etc - + @classmethod diff --git a/lib/python/pyflyby/_interactive.py b/lib/python/pyflyby/_interactive.py index 327e39b5..282143a2 100644 --- a/lib/python/pyflyby/_interactive.py +++ b/lib/python/pyflyby/_interactive.py @@ -758,13 +758,13 @@ def __init__(self, values, ip): dict.__init__(values) self._ip = ip + @property def _potential_imports_list(self): """Collect symbols that could be imported into the namespace. This needs to be executed each time because the context can change, e.g. when in pdb the frames and their namespaces will change.""" - db = None db = ImportDB.interpret_arg(db, target_filename=".") known = db.known_imports diff --git a/meson.build b/meson.build index fa788e9f..9d68cbb3 100644 --- a/meson.build +++ b/meson.build @@ -15,8 +15,7 @@ subdir('lib/python/pyflyby') install_data( ['libexec/pyflyby/colordiff', 'libexec/pyflyby/diff-colorize'], install_dir: py.get_install_dir( - subdir: 'libexec/pyflyby', - pure: true, # Not really pure, these are bash scripts, but we're not compiling them. + subdir: 'pyflyby/libexec/pyflyby', ) ) @@ -30,16 +29,14 @@ install_data( './etc/pyflyby/std.py', ], install_dir: py.get_install_dir( - subdir: 'etc/pyflyby', - pure: true, + subdir: 'pyflyby/etc/pyflyby', ) ) install_data( ['./lib/emacs/pyflyby.el'], install_dir: py.get_install_dir( - subdir: 'share/emacs/site-lisp', - pure: true, + subdir: 'pyflyby/share/emacs/site-lisp', ) ) From 7390cfc9cd7866e2f2946da73c2be673ee697cd1 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 20 Aug 2025 12:42:49 -0700 Subject: [PATCH 26/41] Set lower bound on meson-python to 0.18.0 to enforce PEP639 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 8b8f08f9..8d4d6fcc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [build-system] requires = [ - "meson-python", + "meson-python>=0.18.0", "pybind11>=2.10.4", ] build-backend = "mesonpy" From 91d85193c70ea609d4e5027904bbbfa0c5f41da4 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Thu, 21 Aug 2025 22:41:31 -0700 Subject: [PATCH 27/41] Add caching; progress bars aren't quite right yet --- lib/python/pyflyby/_log.py | 19 +++++-- lib/python/pyflyby/_modules.py | 91 ++++++++++++++++++++++++++++++++-- pyproject.toml | 2 + src/_fast_iter_modules.cpp | 90 +++++++++++++++++++-------------- 4 files changed, 158 insertions(+), 44 deletions(-) diff --git a/lib/python/pyflyby/_log.py b/lib/python/pyflyby/_log.py index f78ac99c..3852e238 100644 --- a/lib/python/pyflyby/_log.py +++ b/lib/python/pyflyby/_log.py @@ -33,10 +33,7 @@ def emit(self, record): # Format (currently a no-op). msg = self.format(record) # Add prefix per line. - if _is_ipython() or _is_interactive(sys.stderr): - prefix = self._interactive_prefix - else: - prefix = self._noninteractive_prefix + prefix = self.get_prefix() msg = ''.join(["%s%s\n" % (prefix, line) for line in msg.splitlines()]) # First, flush stdout, to make sure that stdout and stderr don't get # interleaved. Normally this is automatic, but when stdout is piped, @@ -56,6 +53,20 @@ def emit(self, record): except: self.handleError(record) + def get_prefix(self) -> str: + """Get the appropriate prefix for the current environment. + + Returns + ------- + str + If this is interactive, or an IPython shell, this is the interactive prefix. + Otherwise, return the noninteractive prefix + """ + if _is_ipython() or _is_interactive(sys.stderr): + return self._interactive_prefix + else: + return self._noninteractive_prefix + @contextmanager def HookCtx(self, pre, post): """ diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 0887c9f7..c2b5af46 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -5,15 +5,22 @@ from __future__ import annotations import ast +import appdirs from functools import cached_property, total_ordering, partial +from rich.console import Console +from prompt_toolkit.patch_stdout import patch_stdout +import hashlib import importlib import itertools +import json import os +import pathlib import pkgutil +from rich.progress import Progress from pyflyby._file import FileText, Filename from pyflyby._idents import DottedIdentifier, is_identifier -from pyflyby._log import logger +from pyflyby._log import logger, _PyflybyHandler from pyflyby._util import (ExcludeImplicitCwdFromPathCtx, cmp, memoize, prefixes) from pyflyby._fast_iter_modules import _iter_file_finder_modules @@ -22,7 +29,7 @@ from six import reraise import sys import types -from typing import Any, Dict, Generator +from typing import Any, Dict, Generator, Union class ErrorDuringImportError(ImportError): """ @@ -504,6 +511,84 @@ def containing(cls, identifier): return module SUFFIXES = sorted(importlib.machinery.all_suffixes()) +def _rebuild_cache(importer: Any, cache_file: pathlib.Path) -> list[tuple[str, bool]]: + """Cache and return all modules found by the importer. + + Parameters + ---------- + importer : Any + Importer to use to import modules + cache_file : pathlib.Path + File where the modules should be cached + + Returns + ------- + list[tuple[str, bool]] + List of tuples containing the module name and whether or not it is a package + + """ + label = _PyflybyHandler().get_prefix() + console = Console(file=sys.__stdout__) + fancy_path = format_path(importer.path) + + with Progress(console=console) as progress: + task = progress.add_task( + f"{label}Building module cache for {fancy_path}..." + ) + modules = _iter_file_finder_modules( + importer, + SUFFIXES, + partial(progress.update, task_id=task), + ) + + # # Write the new cache file + progress.update( + task, description=f"{label}Dumping cache updated for {fancy_path}..." + ) + with open(cache_file, 'w') as fp: + json.dump(modules, fp) + + progress.update( + task, description=f"{label}Module cache updated for {fancy_path}" + ) + + return modules + + +def format_path(path: Union[str, pathlib.Path]) -> str: + path = pathlib.Path(path) + home = pathlib.Path.home() + + if path.is_relative_to(home): + return str(pathlib.Path("~").joinpath(path.relative_to(home))) + return str(path) + + +def _cached_module_finder(importer, prefix=''): + if hasattr(importer, 'path'): + mtime = os.stat(importer.path).st_mtime_ns + + cache_dir = pathlib.Path( + appdirs.user_cache_dir(appname='pyflyby', appauthor=False) + ) / hashlib.sha256(str(importer.path).encode()).hexdigest() + cache_file = cache_dir / str(mtime) + + if cache_file.exists(): + with open(cache_file) as fp: + modules = json.load(fp) + else: + cache_dir.mkdir(parents=True, exist_ok=True) + + with patch_stdout(): + modules = _rebuild_cache(importer, cache_file) + + else: + modules = _iter_file_finder_modules(importer, SUFFIXES) + + for module, ispkg in modules: + yield prefix + module, ispkg + + def fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: """Return an iterator over all importable python modules. @@ -515,7 +600,7 @@ def fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: :return: The modules that are importable by python """ pkgutil.iter_importer_modules.register( # type: ignore[attr-defined] - importlib.machinery.FileFinder, partial(_iter_file_finder_modules, suffixes=SUFFIXES) + importlib.machinery.FileFinder, _cached_module_finder ) yield from pkgutil.iter_modules() pkgutil.iter_importer_modules.register( # type: ignore[attr-defined] diff --git a/pyproject.toml b/pyproject.toml index 8d4d6fcc..d58df35f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,8 @@ dependencies = [ "typing_extensions>=4.6; python_version<'3.12'", 'epydoc', 'wheel', # required by epydoc, but not listed as a dependency + 'appdirs', + 'rich', ] [project.urls] Homepage = "https://pypi.org/project/pyflyby/" diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp index 49ee2cfc..faba21d2 100644 --- a/src/_fast_iter_modules.cpp +++ b/src/_fast_iter_modules.cpp @@ -1,5 +1,9 @@ #include "pybind11/cast.h" #include "pybind11/pytypes.h" +#include +#include +#include +#include #include #include #include @@ -9,7 +13,7 @@ namespace py = pybind11; namespace fs = std::filesystem; - +using namespace pybind11::literals; /** * @brief Fast equivalent of `inspect.getmodulename`. @@ -38,54 +42,66 @@ std::string getmodulename(fs::path path, std::vector suffixes) { * * @param importer Importer instance containing an import path. Typically this is an object of type * `importlib.machinery.FileFinder` - * @param prefix A string prefix to affix to the front of all returned modules * @param suffixes Suffixes of valid python modules. Typically this is * `importlib.machinery.all_suffixes()` * @return A vector of tuples containing modules names, and a boolean indicating whether the module * is a package or not */ -std::vector> _iter_file_finder_modules( - py::object importer, std::string prefix, std::vector suffixes +std::vector> +_iter_file_finder_modules( + py::object importer, + std::vector suffixes, + py::object update ) { - std::vector> ret; + std::vector> ret; - // The importer doesn't have a path - py::object path_obj = importer.attr("path"); - if (path_obj.is_none()) { - return ret; - } + // The importer doesn't have a path + py::object path_obj = importer.attr("path"); + if (path_obj.is_none()) { + return ret; + } + + // The importer's path isn't an existing directory + fs::path path = fs::path(py::str(path_obj).cast()); + if (!fs::is_directory(path) || !fs::exists(path)) { + return ret; + } - // The importer's path isn't an existing directory - fs::path path = fs::path(py::str(path_obj).cast()); - if (!fs::is_directory(path) || !fs::exists(path)) { - return ret; + bool has_progress = !update.is_none(); + fs::directory_iterator it = fs::directory_iterator(path); + std::size_t n_items = std::distance(it, fs::directory_iterator{}); + std::size_t i = 0; + + for (auto const &entry : fs::directory_iterator(path)) { + fs::path entry_path = entry.path(); + fs::path filename = entry_path.filename(); + std::string modname = getmodulename(filename, suffixes); + + if (has_progress) { + update("total"_a=n_items, "completed"_a=i); + i++; } - for (auto const& entry : fs::directory_iterator(path)) { - fs::path entry_path = entry.path(); - fs::path filename = entry_path.filename(); - std::string modname = getmodulename(filename, suffixes); + if (modname == "" && fs::is_directory(entry_path) && + filename.string().find(".") == std::string::npos && + fs::is_regular_file(entry_path / "__init__.py") // Is this a package? + ) { + ret.push_back(std::make_tuple(filename.string(), true)); + } else if (modname == "__init__") { + continue; + } else if (modname != "" && modname.find(".") == std::string::npos) { + ret.push_back(std::make_tuple(modname, + false // This is definitely not a package + )); + } + } - if ( - modname == "" - && fs::is_directory(entry_path) - && filename.string().find(".") == std::string::npos - && fs::is_regular_file(entry_path / "__init__.py") // Is this a package? - ) { - ret.push_back(std::make_tuple(prefix + filename.string(), true)); - } else if (modname == "__init__") { - continue; - } else if (modname != "" && modname.find(".") == std::string::npos){ - ret.push_back( - std::make_tuple( - prefix + modname, - false // This is definitely not a package - ) - ); - } + // Close out any open progress bars + if (has_progress) { + update("total"_a=n_items, "completed"_a=n_items); } - return ret; + return ret; } PYBIND11_MODULE(_fast_iter_modules, m) { @@ -95,8 +111,8 @@ PYBIND11_MODULE(_fast_iter_modules, m) { &_iter_file_finder_modules, "A fast implementation of pkgutil._iter_file_finder_modules(importer, prefix='')", py::arg("importer"), - py::arg("prefix") = py::str(""), py::arg("suffixes") = std::make_tuple(".py", ".pyc"), + py::arg("update") = py::none(), py::return_value_policy::take_ownership ); } From a4dd90792e67310b9562e0642520b2d7ce499ba3 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 22 Aug 2025 11:04:01 -0700 Subject: [PATCH 28/41] Don't bother with progress bars --- lib/python/pyflyby/_modules.py | 78 +++++++++++----------------------- pyproject.toml | 1 - src/_fast_iter_modules.cpp | 22 +--------- 3 files changed, 25 insertions(+), 76 deletions(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index c2b5af46..4350ee90 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -6,8 +6,7 @@ import ast import appdirs -from functools import cached_property, total_ordering, partial -from rich.console import Console +from functools import cached_property, total_ordering from prompt_toolkit.patch_stdout import patch_stdout import hashlib import importlib @@ -16,11 +15,10 @@ import os import pathlib import pkgutil -from rich.progress import Progress from pyflyby._file import FileText, Filename from pyflyby._idents import DottedIdentifier, is_identifier -from pyflyby._log import logger, _PyflybyHandler +from pyflyby._log import logger from pyflyby._util import (ExcludeImplicitCwdFromPathCtx, cmp, memoize, prefixes) from pyflyby._fast_iter_modules import _iter_file_finder_modules @@ -511,50 +509,6 @@ def containing(cls, identifier): return module SUFFIXES = sorted(importlib.machinery.all_suffixes()) -def _rebuild_cache(importer: Any, cache_file: pathlib.Path) -> list[tuple[str, bool]]: - """Cache and return all modules found by the importer. - - Parameters - ---------- - importer : Any - Importer to use to import modules - cache_file : pathlib.Path - File where the modules should be cached - - Returns - ------- - list[tuple[str, bool]] - List of tuples containing the module name and whether or not it is a package - - """ - label = _PyflybyHandler().get_prefix() - console = Console(file=sys.__stdout__) - fancy_path = format_path(importer.path) - - with Progress(console=console) as progress: - task = progress.add_task( - f"{label}Building module cache for {fancy_path}..." - ) - modules = _iter_file_finder_modules( - importer, - SUFFIXES, - partial(progress.update, task_id=task), - ) - - # # Write the new cache file - progress.update( - task, description=f"{label}Dumping cache updated for {fancy_path}..." - ) - with open(cache_file, 'w') as fp: - json.dump(modules, fp) - - progress.update( - task, description=f"{label}Module cache updated for {fancy_path}" - ) - - return modules - - def format_path(path: Union[str, pathlib.Path]) -> str: path = pathlib.Path(path) home = pathlib.Path.home() @@ -564,14 +518,26 @@ def format_path(path: Union[str, pathlib.Path]) -> str: return str(path) -def _cached_module_finder(importer, prefix=''): - if hasattr(importer, 'path'): - mtime = os.stat(importer.path).st_mtime_ns +def _cached_module_finder(importer: importlib.machinery.FileFinder, prefix: str = ''): + """Yield the modules found by the importer. + + The importer path's mtime is recorded; if the path and mtime have a corresponding + cache file, the modules recorded in the cache file are returned. Otherwise, the + cache is rebuilt. + + Parameters + ---------- + importer : importlib.machinery.FileFinder + FileFinder importer that points to a path under which imports can be found + prefix : str + String to affix to the beginning of each module name + """ + if hasattr(importer, 'path'): cache_dir = pathlib.Path( appdirs.user_cache_dir(appname='pyflyby', appauthor=False) ) / hashlib.sha256(str(importer.path).encode()).hexdigest() - cache_file = cache_dir / str(mtime) + cache_file = cache_dir / str(os.stat(importer.path).st_mtime_ns) if cache_file.exists(): with open(cache_file) as fp: @@ -579,8 +545,12 @@ def _cached_module_finder(importer, prefix=''): else: cache_dir.mkdir(parents=True, exist_ok=True) - with patch_stdout(): - modules = _rebuild_cache(importer, cache_file) + with patch_stdout(raw=True): + logger.info(f"Rebuilding cache for {format_path(importer.path)}...") + + modules = _iter_file_finder_modules(importer, SUFFIXES) + with open(cache_file, 'w') as fp: + json.dump(modules, fp) else: modules = _iter_file_finder_modules(importer, SUFFIXES) diff --git a/pyproject.toml b/pyproject.toml index d58df35f..cd6554f8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,7 +32,6 @@ dependencies = [ 'epydoc', 'wheel', # required by epydoc, but not listed as a dependency 'appdirs', - 'rich', ] [project.urls] Homepage = "https://pypi.org/project/pyflyby/" diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp index faba21d2..a8e36f43 100644 --- a/src/_fast_iter_modules.cpp +++ b/src/_fast_iter_modules.cpp @@ -1,9 +1,5 @@ #include "pybind11/cast.h" #include "pybind11/pytypes.h" -#include -#include -#include -#include #include #include #include @@ -50,8 +46,7 @@ std::string getmodulename(fs::path path, std::vector suffixes) { std::vector> _iter_file_finder_modules( py::object importer, - std::vector suffixes, - py::object update + std::vector suffixes ) { std::vector> ret; @@ -67,20 +62,11 @@ _iter_file_finder_modules( return ret; } - bool has_progress = !update.is_none(); - fs::directory_iterator it = fs::directory_iterator(path); - std::size_t n_items = std::distance(it, fs::directory_iterator{}); - std::size_t i = 0; - for (auto const &entry : fs::directory_iterator(path)) { fs::path entry_path = entry.path(); fs::path filename = entry_path.filename(); std::string modname = getmodulename(filename, suffixes); - if (has_progress) { - update("total"_a=n_items, "completed"_a=i); - i++; - } if (modname == "" && fs::is_directory(entry_path) && filename.string().find(".") == std::string::npos && @@ -96,11 +82,6 @@ _iter_file_finder_modules( } } - // Close out any open progress bars - if (has_progress) { - update("total"_a=n_items, "completed"_a=n_items); - } - return ret; } @@ -112,7 +93,6 @@ PYBIND11_MODULE(_fast_iter_modules, m) { "A fast implementation of pkgutil._iter_file_finder_modules(importer, prefix='')", py::arg("importer"), py::arg("suffixes") = std::make_tuple(".py", ".pyc"), - py::arg("update") = py::none(), py::return_value_policy::take_ownership ); } From 847d9ab778f3918245f8ee5c6672f8d0e62c05ae Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 22 Aug 2025 12:16:01 -0700 Subject: [PATCH 29/41] Add a function to force the cache to be rebuilt --- lib/python/pyflyby/_modules.py | 117 +++++++++++++++++++++++++-------- 1 file changed, 91 insertions(+), 26 deletions(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 4350ee90..a54b7510 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -4,10 +4,9 @@ from __future__ import annotations -import ast import appdirs +import ast from functools import cached_property, total_ordering -from prompt_toolkit.patch_stdout import patch_stdout import hashlib import importlib import itertools @@ -15,15 +14,20 @@ import os import pathlib import pkgutil +from prompt_toolkit.patch_stdout \ + import patch_stdout +import textwrap +from pyflyby._fast_iter_modules \ + import _iter_file_finder_modules from pyflyby._file import FileText, Filename from pyflyby._idents import DottedIdentifier, is_identifier from pyflyby._log import logger from pyflyby._util import (ExcludeImplicitCwdFromPathCtx, cmp, memoize, prefixes) -from pyflyby._fast_iter_modules import _iter_file_finder_modules import re +import shutil from six import reraise import sys import types @@ -37,6 +41,42 @@ class ErrorDuringImportError(ImportError): exist. """ +def rebuild_import_cache(): + """Force the import cache to be rebuilt. + + The cache is deleted before calling _fast_iter_modules, which repopulates the cache. + """ + for path in pathlib.Path( + appdirs.user_cache_dir(appname='pyflyby', appauthor=False) + ).iterdir(): + _remove_import_cache_dir(path) + _fast_iter_modules() + + +def _remove_import_cache_dir(path: pathlib.Path): + """Remove an import cache directory. + + Import cache directories exist in /pyflyby/, and they should + contain just a single file which itself contains a JSON blob of cached import names. + We therefore only delete the requested path if it is a directory. + + Parameters + ---------- + path : pathlib.Path + Import cache directory path to remove + """ + if path.is_dir(): + # Only directories are valid import cache entries + try: + shutil.rmtree(str(path)) + except Exception as e: + with patch_stdout(raw=True): + logger.error( + f"Failed to remove cache directory at {path} - please " + "consider removing this directory manually. Error:\n" + f"{textwrap.indent(str(e), prefix=' ')}" + ) + @memoize def import_module(module_name): @@ -301,7 +341,7 @@ def list() -> list[str]: :return: A list of all importable module names """ with ExcludeImplicitCwdFromPathCtx(): - return [mod.name for mod in fast_iter_modules() if is_identifier(mod.name)] + return [mod.name for mod in _fast_iter_modules() if is_identifier(mod.name)] @cached_property def submodules(self): @@ -508,8 +548,23 @@ def containing(cls, identifier): logger.debug("Imported %r to get %r", module, identifier) return module -SUFFIXES = sorted(importlib.machinery.all_suffixes()) -def format_path(path: Union[str, pathlib.Path]) -> str: + +def _format_path(path: Union[str, pathlib.Path]) -> str: + """Format a path for printing as a log message. + + If the path is a child of $HOME, the prefix is replaced with "~" for brevity. + Otherwise the original path is returned. + + Parameters + ---------- + path : Union[str, pathlib.Path] + Path to format + + Returns + ------- + str + Formatted output path + """ path = pathlib.Path(path) home = pathlib.Path.home() @@ -518,7 +573,12 @@ def format_path(path: Union[str, pathlib.Path]) -> str: return str(path) -def _cached_module_finder(importer: importlib.machinery.FileFinder, prefix: str = ''): +SUFFIXES = sorted(importlib.machinery.all_suffixes()) + + +def _cached_module_finder( + importer: importlib.machinery.FileFinder, prefix: str = "" +) -> Generator[tuple[str, bool], None, None]: """Yield the modules found by the importer. The importer path's mtime is recorded; if the path and mtime have a corresponding @@ -532,34 +592,39 @@ def _cached_module_finder(importer: importlib.machinery.FileFinder, prefix: str prefix : str String to affix to the beginning of each module name + Returns + ------- + Generator[tuple[str, bool], None, None] + Tuples containing (prefix+module name, a bool indicating whether the module is a + package or not) """ - if hasattr(importer, 'path'): - cache_dir = pathlib.Path( - appdirs.user_cache_dir(appname='pyflyby', appauthor=False) - ) / hashlib.sha256(str(importer.path).encode()).hexdigest() - cache_file = cache_dir / str(os.stat(importer.path).st_mtime_ns) - - if cache_file.exists(): - with open(cache_file) as fp: - modules = json.load(fp) - else: - cache_dir.mkdir(parents=True, exist_ok=True) - - with patch_stdout(raw=True): - logger.info(f"Rebuilding cache for {format_path(importer.path)}...") + cache_dir = pathlib.Path( + appdirs.user_cache_dir(appname='pyflyby', appauthor=False) + ) / hashlib.sha256(str(importer.path).encode()).hexdigest() + cache_file = cache_dir / str(os.stat(importer.path).st_mtime_ns) + + if cache_file.exists(): + with open(cache_file) as fp: + modules = json.load(fp) + else: + # Generate the cache dir if it doesn't exist, and remove any existing cache + # files for the given import path + cache_dir.mkdir(parents=True, exist_ok=True) + for path in cache_dir.iterdir(): + _remove_import_cache_dir(path) - modules = _iter_file_finder_modules(importer, SUFFIXES) - with open(cache_file, 'w') as fp: - json.dump(modules, fp) + with patch_stdout(raw=True): + logger.info(f"Rebuilding cache for {_format_path(importer.path)}...") - else: modules = _iter_file_finder_modules(importer, SUFFIXES) + with open(cache_file, 'w') as fp: + json.dump(modules, fp) for module, ispkg in modules: yield prefix + module, ispkg -def fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: +def _fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: """Return an iterator over all importable python modules. This function patches `pkgutil.iter_importer_modules` for From d61b636b191ece1696ad72b9ef5377a594e7a7db Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 22 Aug 2025 12:33:55 -0700 Subject: [PATCH 30/41] Export rebuild_import_cache at top level --- lib/python/pyflyby/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/python/pyflyby/__init__.py b/lib/python/pyflyby/__init__.py index 92fcc162..5b3ee656 100644 --- a/lib/python/pyflyby/__init__.py +++ b/lib/python/pyflyby/__init__.py @@ -31,6 +31,7 @@ unload_ipython_extension) from pyflyby._livepatch import livepatch, xreload from pyflyby._log import logger +from pyflyby._modules import rebuild_import_cache from pyflyby._parse import PythonBlock, PythonStatement from pyflyby._saveframe import saveframe from pyflyby._saveframe_reader \ From d8f312ef1914ddcd8c8b19090888666e9b1e60ed Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 22 Aug 2025 12:37:20 -0700 Subject: [PATCH 31/41] Cleanup --- lib/python/pyflyby/_importdb.py | 2 +- lib/python/pyflyby/_interactive.py | 4 ++-- lib/python/pyflyby/_log.py | 19 ++++--------------- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/lib/python/pyflyby/_importdb.py b/lib/python/pyflyby/_importdb.py index 54f44893..fd69bc9c 100644 --- a/lib/python/pyflyby/_importdb.py +++ b/lib/python/pyflyby/_importdb.py @@ -219,7 +219,7 @@ def __new__(cls, *args): return cls._from_data(arg, [], [], []) return cls._from_args(arg) # PythonBlock, Filename, etc - + @classmethod diff --git a/lib/python/pyflyby/_interactive.py b/lib/python/pyflyby/_interactive.py index 282143a2..09eeaae1 100644 --- a/lib/python/pyflyby/_interactive.py +++ b/lib/python/pyflyby/_interactive.py @@ -22,7 +22,7 @@ auto_import, auto_import_symbol, clear_failed_imports_cache) -from pyflyby._dynimp import (inject as inject_dynamic_import, +from pyflyby._dynimp import (inject as inject_dynamic_import, PYFLYBY_LAZY_LOAD_PREFIX) from pyflyby._comms import (initialize_comms, remove_comms, send_comm_message, MISSING_IMPORTS) @@ -758,13 +758,13 @@ def __init__(self, values, ip): dict.__init__(values) self._ip = ip - @property def _potential_imports_list(self): """Collect symbols that could be imported into the namespace. This needs to be executed each time because the context can change, e.g. when in pdb the frames and their namespaces will change.""" + db = None db = ImportDB.interpret_arg(db, target_filename=".") known = db.known_imports diff --git a/lib/python/pyflyby/_log.py b/lib/python/pyflyby/_log.py index 3852e238..f78ac99c 100644 --- a/lib/python/pyflyby/_log.py +++ b/lib/python/pyflyby/_log.py @@ -33,7 +33,10 @@ def emit(self, record): # Format (currently a no-op). msg = self.format(record) # Add prefix per line. - prefix = self.get_prefix() + if _is_ipython() or _is_interactive(sys.stderr): + prefix = self._interactive_prefix + else: + prefix = self._noninteractive_prefix msg = ''.join(["%s%s\n" % (prefix, line) for line in msg.splitlines()]) # First, flush stdout, to make sure that stdout and stderr don't get # interleaved. Normally this is automatic, but when stdout is piped, @@ -53,20 +56,6 @@ def emit(self, record): except: self.handleError(record) - def get_prefix(self) -> str: - """Get the appropriate prefix for the current environment. - - Returns - ------- - str - If this is interactive, or an IPython shell, this is the interactive prefix. - Otherwise, return the noninteractive prefix - """ - if _is_ipython() or _is_interactive(sys.stderr): - return self._interactive_prefix - else: - return self._noninteractive_prefix - @contextmanager def HookCtx(self, pre, post): """ From ee6ae7de0c41007b6d84fcfd49eff3cba12c4c86 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 22 Aug 2025 15:19:32 -0700 Subject: [PATCH 32/41] Expand import caching test --- tests/test_modules.py | 65 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/test_modules.py b/tests/test_modules.py index 0893c6e5..dfd4c43c 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -6,10 +6,15 @@ +from unittest import mock +import pathlib +import hashlib +import json import logging.handlers from pyflyby._file import Filename from pyflyby._idents import DottedIdentifier -from pyflyby._modules import ModuleHandle, fast_iter_modules +from pyflyby._log import logger +from pyflyby._modules import ModuleHandle, _fast_iter_modules, _iter_file_finder_modules from pkgutil import iter_modules import re import subprocess @@ -112,9 +117,65 @@ def test_filename_noload_1(modname): assert ret.returncode != 120, f"{modname} in sys.modules at startup" assert ret.returncode == 0, (ret, ret.stdout, ret.stderr) + def test_fast_iter_modules(): """Test that the cpp extension finds the same modules as pkgutil.iter_modules.""" - fast = sorted(list(fast_iter_modules()), key=lambda x: x.name) + fast = sorted(list(_fast_iter_modules()), key=lambda x: x.name) slow = sorted(list(iter_modules()), key=lambda x: x.name) assert fast == slow + + +@mock.patch("appdirs.user_cache_dir") +def test_import_cache(mock_user_cache_dir, tmp_path): + """Test that the import cache is built when iterating modules. + + Also: + - Check that each path mentioned in the logs appears (sha256-encoded) in the cache + - The first time generating the import cache, _iter_file_finder_modules is called + - Subsequent calls use the cached modules + """ + + mock_user_cache_dir.return_value = tmp_path + + assert len(list(tmp_path.iterdir())) == 0 + with ( + mock.patch("pyflyby._modules.logger", wraps=logger) as mock_logger, + mock.patch( + "pyflyby._modules._iter_file_finder_modules", + wraps=_iter_file_finder_modules, + ) as mock_iffm, + ): + list(_fast_iter_modules()) + + paths = [str(path.name) for path in tmp_path.iterdir()] + n_cached_paths = len(paths) + n_log_messages = len(mock_logger.info.call_args_list) + + # On the first call, log messages should be generated for each import path. Check + # that _iter_file_finder_modules was called once for each cached path. + assert (n_cached_paths == n_log_messages) and n_cached_paths > 0 + assert len(mock_iffm.call_args_list) == n_cached_paths + assert "Rebuilding cache for " in mock_logger.info.call_args.args[0] + for call_args in mock_logger.info.call_args_list: + # Grab the path names from the log messages; make sure the sha256 checksum + # can be found in the paths of the cache directory + path = pathlib.Path( + call_args.args[0].lstrip("Rebuilding cache for ").rstrip("...") + ).expanduser() + assert hashlib.sha256(str(path).encode()).hexdigest() in paths + + with ( + mock.patch("pyflyby._modules.logger", wraps=logger) as mock_logger, + mock.patch( + "pyflyby._modules._iter_file_finder_modules", + wraps=_iter_file_finder_modules, + ) as mock_iffm, + ): + list(_fast_iter_modules()) + + # On the second call, no additional messages should be emitted because the cache has + # already been built. Check that _iter_file_finder_modules was never called. + n_log_messages = len(mock_logger.info.call_args_list) + assert n_log_messages == 0 + mock_iffm.assert_not_called() From 9ade0cd4fd928986ba81f125f29832059c06b17c Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 22 Aug 2025 15:25:23 -0700 Subject: [PATCH 33/41] Check that updating the mtime of an importer path regenerates the cache --- tests/test_modules.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_modules.py b/tests/test_modules.py index dfd4c43c..9f921f5c 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -134,6 +134,8 @@ def test_import_cache(mock_user_cache_dir, tmp_path): - Check that each path mentioned in the logs appears (sha256-encoded) in the cache - The first time generating the import cache, _iter_file_finder_modules is called - Subsequent calls use the cached modules + - If the mtime of one of the importer paths is updated, the corresponding + cache file gets regenerated """ mock_user_cache_dir.return_value = tmp_path @@ -179,3 +181,20 @@ def test_import_cache(mock_user_cache_dir, tmp_path): n_log_messages = len(mock_logger.info.call_args_list) assert n_log_messages == 0 mock_iffm.assert_not_called() + + # Update the mtime of one of the importer paths + path.touch() + with ( + mock.patch("pyflyby._modules.logger", wraps=logger) as mock_logger, + mock.patch( + "pyflyby._modules._iter_file_finder_modules", + wraps=_iter_file_finder_modules, + ) as mock_iffm, + ): + list(_fast_iter_modules()) + + # Only one path should have been updated and only 1 message logged. The number + # of cache directories should not change. + assert len(mock_logger.info.call_args_list) == 1 + assert len(list(tmp_path.iterdir())) == n_cached_paths + mock_iffm.assert_called_once() From 8ae7d628aa4801dc346b4bdac7ecdcc4d221e019 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 22 Aug 2025 15:28:58 -0700 Subject: [PATCH 34/41] Add `appdirs` to autodoc_mock_imports; remove unused import --- doc/conf.py | 3 ++- tests/test_modules.py | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/conf.py b/doc/conf.py index 73c739ca..0c5ec40b 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -44,7 +44,8 @@ def find_version(): } autodoc_mock_imports = [ - "pyflyby._fast_iter_modules" + "pyflyby._fast_iter_modules", + "appdirs", ] html_theme_options = { diff --git a/tests/test_modules.py b/tests/test_modules.py index 9f921f5c..0b74ea24 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -9,7 +9,6 @@ from unittest import mock import pathlib import hashlib -import json import logging.handlers from pyflyby._file import Filename from pyflyby._idents import DottedIdentifier From c9a467c2d79dbc7c8e4b085a5e3cc299e3c08605 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Fri, 22 Aug 2025 15:33:19 -0700 Subject: [PATCH 35/41] Add `prompt-toolkit` as dependency --- doc/conf.py | 1 + pyproject.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/doc/conf.py b/doc/conf.py index 0c5ec40b..365b01a7 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -46,6 +46,7 @@ def find_version(): autodoc_mock_imports = [ "pyflyby._fast_iter_modules", "appdirs", + "prompt_toolkit", ] html_theme_options = { diff --git a/pyproject.toml b/pyproject.toml index cd6554f8..73e42382 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,6 +29,7 @@ dependencies = [ "six", "toml", "typing_extensions>=4.6; python_version<'3.12'", + 'prompt_toolkit', 'epydoc', 'wheel', # required by epydoc, but not listed as a dependency 'appdirs', From 4e7cbf56be7731f41ff6416df3edce1ca33fa7e2 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Mon, 25 Aug 2025 16:40:10 -0700 Subject: [PATCH 36/41] Fix tests broken by new log messages --- tests/test_interactive.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_interactive.py b/tests/test_interactive.py index 5475cfaa..b97dadd2 100644 --- a/tests/test_interactive.py +++ b/tests/test_interactive.py @@ -2361,6 +2361,7 @@ def f_68421204(): return 'good' """) ipython(""" In [1]: import pyflyby; pyflyby.enable_auto_importer() + [PYFLYBY] Rebuilding cache for ... In [2]: m18908697_\tfoo.f_68421204() [PYFLYBY] import m18908697_foo Out[2]: 'good' @@ -2377,8 +2378,9 @@ def f_76313558_59577191(): return 'ok' """) ipython(""" In [1]: import pyflyby; pyflyby.enable_auto_importer() - In [2]: m51145108_\tfoo.f_76313558_\t + [PYFLYBY] Rebuilding cache for ... [PYFLYBY] import m51145108_foo + In [2]: m51145108_\tfoo.f_76313558_\t In [2]: m51145108_foo.f_76313558_59577191() Out[2]: 'ok' """, PYTHONPATH=tmp.dir, frontend=frontend) From 7a45d5b19df2a685598bd964708d8704cd6ffd04 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Tue, 26 Aug 2025 11:20:56 -0700 Subject: [PATCH 37/41] Disable test broken due to log messages --- tests/test_interactive.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_interactive.py b/tests/test_interactive.py index b97dadd2..3ae33c2e 100644 --- a/tests/test_interactive.py +++ b/tests/test_interactive.py @@ -3858,6 +3858,12 @@ def test_debug_tab_completion_db_1(frontend): """, frontend=frontend) +@pytest.mark.skip( + reason=( + "ipdb runs commands in a thread, which breaks prompt_toolkit.patch_stdout. " + "Turn this back on if a solution can be found." + ) +) @pytest.mark.skipif(_SUPPORTS_TAB_AUTO_IMPORT, reason='Autoimport on Tab requires IPython 9.3+') def test_debug_tab_completion_module_1(frontend, tmp): # Verify that tab completion on module names works. @@ -3880,6 +3886,12 @@ def test_debug_tab_completion_module_1(frontend, tmp): """, PYTHONPATH=tmp.dir, frontend=frontend) +@pytest.mark.skip( + reason=( + "ipdb runs commands in a thread, which breaks prompt_toolkit.patch_stdout. " + "Turn this back on if a solution can be found." + ) +) @pytest.mark.skipif(_SUPPORTS_TAB_AUTO_IMPORT, reason='Autoimport on Tab requires IPython 9.3+') @retry def test_debug_tab_completion_multiple_1(frontend, tmp): From e81442d89c1be028c02fd9c7c1e200c52005c0c3 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Tue, 26 Aug 2025 13:31:49 -0700 Subject: [PATCH 38/41] Remove patch_stdout calls; that's a problem for another time --- lib/python/pyflyby/_modules.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index a54b7510..6fef6e77 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -14,8 +14,6 @@ import os import pathlib import pkgutil -from prompt_toolkit.patch_stdout \ - import patch_stdout import textwrap from pyflyby._fast_iter_modules \ @@ -70,12 +68,11 @@ def _remove_import_cache_dir(path: pathlib.Path): try: shutil.rmtree(str(path)) except Exception as e: - with patch_stdout(raw=True): - logger.error( - f"Failed to remove cache directory at {path} - please " - "consider removing this directory manually. Error:\n" - f"{textwrap.indent(str(e), prefix=' ')}" - ) + logger.error( + f"Failed to remove cache directory at {path} - please " + "consider removing this directory manually. Error:\n" + f"{textwrap.indent(str(e), prefix=' ')}" + ) @memoize @@ -613,8 +610,7 @@ def _cached_module_finder( for path in cache_dir.iterdir(): _remove_import_cache_dir(path) - with patch_stdout(raw=True): - logger.info(f"Rebuilding cache for {_format_path(importer.path)}...") + logger.info(f"Rebuilding cache for {_format_path(importer.path)}...") modules = _iter_file_finder_modules(importer, SUFFIXES) with open(cache_file, 'w') as fp: From 3d4a9add608b5273f6c92cc0f8c1afb25abe71e7 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Tue, 26 Aug 2025 16:14:58 -0700 Subject: [PATCH 39/41] Try using two scenarios --- tests/test_interactive.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/test_interactive.py b/tests/test_interactive.py index 3ae33c2e..029887cf 100644 --- a/tests/test_interactive.py +++ b/tests/test_interactive.py @@ -2472,14 +2472,24 @@ def test_complete_symbol_eval_1(evaluation): @pytest.mark.skipif(_SUPPORTS_TAB_AUTO_IMPORT, reason='Autoimport on Tab requires IPython 9.3+') @pytest.mark.parametrize('evaluation', _TESTED_EVALUATION_SETTINGS) def test_complete_symbol_eval_autoimport_1(frontend, evaluation): - ipython(f""" + template = """ In [1]: import pyflyby; pyflyby.enable_auto_importer() - In [2]: %config IPCompleter.{evaluation} - In [3]: os.sep.strip().lst\t - [PYFLYBY] import os - In [3]: os.sep.strip().lst\trip + In [2]: %config IPCompleter.{0} + In [3]: os.sep.strip().lst\t{1} Out[3]: - """, frontend=frontend) + """ + + scenario_a = """ + [PYFLYBY] import os + In [3]: os.sep.strip().lst\trip""" + scenario_b = """ + [PYFLYBY] import os + In [3]: os.sep.strip().lstrip""" + + try: + ipython(template.format(evaluation, scenario_a), frontend=frontend) + except pytest.fail.Exception: + ipython(template.format(evaluation, scenario_b), frontend=frontend) @pytest.mark.skipif(_SUPPORTS_TAB_AUTO_IMPORT, reason='Autoimport on Tab requires IPython 9.3+') From fadc7289ba93c782e5052dc54cf620a6dfa704c1 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 27 Aug 2025 12:46:48 -0700 Subject: [PATCH 40/41] Try stripping out log lines; can't do anything else since subprocess... --- tests/test_interactive.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/tests/test_interactive.py b/tests/test_interactive.py index 029887cf..3e67e9e1 100644 --- a/tests/test_interactive.py +++ b/tests/test_interactive.py @@ -1395,6 +1395,14 @@ def _clean_ipython_output(result): # Remove code to clear to end of line. This is done here instead of in # decode() because _wait_nonce looks for this code. result = result.replace(b"\x1b[K", b"") + # Remove cache rebuild log messages + lines = [] + for line in result.split(b"\n"): + if b"[PYFLYBY] Rebuilding cache for" not in line: + lines.append(line) + + result = b"\n".join(lines) + # result = re.sub(rb"\[PYFLYBY\] Rebuilding cache for .*\.\.\.\n", b"", result, flags=re.M).strip() result = result.lstrip() if _IPYTHON_VERSION >= (5,): # and _IPYTHON_VERSION <= (8,): # In IPython 5 kernel/console/etc, it seems to be impossible to turn @@ -2361,7 +2369,6 @@ def f_68421204(): return 'good' """) ipython(""" In [1]: import pyflyby; pyflyby.enable_auto_importer() - [PYFLYBY] Rebuilding cache for ... In [2]: m18908697_\tfoo.f_68421204() [PYFLYBY] import m18908697_foo Out[2]: 'good' @@ -2378,7 +2385,6 @@ def f_76313558_59577191(): return 'ok' """) ipython(""" In [1]: import pyflyby; pyflyby.enable_auto_importer() - [PYFLYBY] Rebuilding cache for ... [PYFLYBY] import m51145108_foo In [2]: m51145108_\tfoo.f_76313558_\t In [2]: m51145108_foo.f_76313558_59577191() @@ -3868,12 +3874,6 @@ def test_debug_tab_completion_db_1(frontend): """, frontend=frontend) -@pytest.mark.skip( - reason=( - "ipdb runs commands in a thread, which breaks prompt_toolkit.patch_stdout. " - "Turn this back on if a solution can be found." - ) -) @pytest.mark.skipif(_SUPPORTS_TAB_AUTO_IMPORT, reason='Autoimport on Tab requires IPython 9.3+') def test_debug_tab_completion_module_1(frontend, tmp): # Verify that tab completion on module names works. @@ -3896,12 +3896,6 @@ def test_debug_tab_completion_module_1(frontend, tmp): """, PYTHONPATH=tmp.dir, frontend=frontend) -@pytest.mark.skip( - reason=( - "ipdb runs commands in a thread, which breaks prompt_toolkit.patch_stdout. " - "Turn this back on if a solution can be found." - ) -) @pytest.mark.skipif(_SUPPORTS_TAB_AUTO_IMPORT, reason='Autoimport on Tab requires IPython 9.3+') @retry def test_debug_tab_completion_multiple_1(frontend, tmp): From 714de2c67bebeb7151c1b362974d7fe5860245c8 Mon Sep 17 00:00:00 2001 From: pdmurray Date: Wed, 27 Aug 2025 14:45:31 -0700 Subject: [PATCH 41/41] Add env variable for suppressing pyflyby cache rebuild log messages --- lib/python/pyflyby/_modules.py | 3 ++- tests/test_interactive.py | 9 +-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 6fef6e77..209f8077 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -610,7 +610,8 @@ def _cached_module_finder( for path in cache_dir.iterdir(): _remove_import_cache_dir(path) - logger.info(f"Rebuilding cache for {_format_path(importer.path)}...") + if os.environ.get("PYFLYBY_SUPPRESS_CACHE_REBUILD_LOGS", 0) != "1": + logger.info(f"Rebuilding cache for {_format_path(importer.path)}...") modules = _iter_file_finder_modules(importer, SUFFIXES) with open(cache_file, 'w') as fp: diff --git a/tests/test_interactive.py b/tests/test_interactive.py index 3e67e9e1..ced14f57 100644 --- a/tests/test_interactive.py +++ b/tests/test_interactive.py @@ -859,6 +859,7 @@ def IPythonCtx(prog="ipython", try: # Prepare environment variables. env = {} + env["PYFLYBY_SUPPRESS_CACHE_REBUILD_LOGS"] = "1" env["PYFLYBY_PATH"] = PYFLYBY_PATH env["PYFLYBY_LOG_LEVEL"] = PYFLYBY_LOG_LEVEL env["PYTHONPATH"] = _build_pythonpath(PYTHONPATH) @@ -1395,14 +1396,6 @@ def _clean_ipython_output(result): # Remove code to clear to end of line. This is done here instead of in # decode() because _wait_nonce looks for this code. result = result.replace(b"\x1b[K", b"") - # Remove cache rebuild log messages - lines = [] - for line in result.split(b"\n"): - if b"[PYFLYBY] Rebuilding cache for" not in line: - lines.append(line) - - result = b"\n".join(lines) - # result = re.sub(rb"\[PYFLYBY\] Rebuilding cache for .*\.\.\.\n", b"", result, flags=re.M).strip() result = result.lstrip() if _IPYTHON_VERSION >= (5,): # and _IPYTHON_VERSION <= (8,): # In IPython 5 kernel/console/etc, it seems to be impossible to turn