Prepare sage.doctest for namespace packages
Closed this issue · 30 comments
The Sage doctester decides whether a Python file is part of a package based on the presence of __init__.py files. This is no longer appropriate because PEP 420 implicit namespace packages do not have __init__.py files; see #32501.
We implement a new function sage.misc.package_dir.is_package_or_sage_namespace_package_dir, which we will also use in #28925.
This change also removes a runtime dependency of the doctester on Cython, see #33029.
CC: @tobiasdiez @kwankyu @kiwifb @antonio-rojas @seblabbe
Component: doctest framework
Author: Matthias Koeppe
Branch/Commit: 5adae47
Reviewer: Tobias Diez, Kwankyu Lee
Issue created by migration from https://trac.sagemath.org/ticket/33033
Description changed:
---
+++
@@ -1,2 +1,2 @@
-The behavior of the Sage doctester decides whether a Python file is part of package based on the presence of `__init__.py` files. This is no longer appropriate because PEP 420 implicit namespace packages do not have `__init__.py` files.
+The Sage doctester decides whether a Python file is part of package based on the presence of `__init__.py` files. This is no longer appropriate because PEP 420 implicit namespace packages do not have `__init__.py` files.
Description changed:
---
+++
@@ -1,2 +1,2 @@
-The Sage doctester decides whether a Python file is part of package based on the presence of `__init__.py` files. This is no longer appropriate because PEP 420 implicit namespace packages do not have `__init__.py` files.
+The Sage doctester decides whether a Python file is part of a package based on the presence of `__init__.py` files. This is no longer appropriate because PEP 420 implicit namespace packages do not have `__init__.py` files.
Commit: ab23a14
Branch pushed to git repo; I updated commit sha1. New commits:
ab23a14 | src/sage/doctest/sources.py: Use is_package_or_sage_namespace_package_dir |
Author: Matthias Koeppe
Description changed:
---
+++
@@ -1,2 +1,4 @@
-The Sage doctester decides whether a Python file is part of a package based on the presence of `__init__.py` files. This is no longer appropriate because PEP 420 implicit namespace packages do not have `__init__.py` files.
+The Sage doctester decides whether a Python file is part of a package based on the presence of `__init__.py` files. This is no longer appropriate because PEP 420 implicit namespace packages do not have `__init__.py` files; see #32501.
+This change also removes a runtime dependency of the doctester on Cython, see #33029.
+A few nitpicks from my side, otherwise looks good (codewise, didn't try it locally).
The explicit bool in sagemath/sagetrac-mirror@develop...u/mkoeppe/prepare_sage_doctest_for_namespace_packages#diff-944be831a2285f89611252d94988abde763acd41fd84353c5b46c01df443dc82L680-L683 can probably be removed (at least according to the comment that had been there).
sage: d = os.path.dirname(sage.cpython.file); d
For readability, I would propose to use dir or directory instead of the single-letter variable name.
Thanks, the changes look good to me!
Reviewer: Tobias Diez, ...
Branch pushed to git repo; I updated commit sha1. New commits:
19e93f0 | is_package_or_sage_namespace_package_dir: Make directories with __init__.pxd package directories |
Description changed:
---
+++
@@ -1,4 +1,7 @@
The Sage doctester decides whether a Python file is part of a package based on the presence of `__init__.py` files. This is no longer appropriate because PEP 420 implicit namespace packages do not have `__init__.py` files; see #32501.
+
+We implement a new function `sage.misc.namespace_package.is_package_or_sage_namespace_package_dir`, which we will also use in #28925.
This change also removes a runtime dependency of the doctester on Cython, see #33029.
+Turns out that this module is not the best place for the new function because the function will need to be packaged in the distribution sagemath-environment.
Branch pushed to git repo; I updated commit sha1. New commits:
5808f3d | src/sage/misc/package_dir.py: New file for is_package_or_sage_namespace_package_dir |
Description changed:
---
+++
@@ -1,6 +1,6 @@
The Sage doctester decides whether a Python file is part of a package based on the presence of `__init__.py` files. This is no longer appropriate because PEP 420 implicit namespace packages do not have `__init__.py` files; see #32501.
-We implement a new function `sage.misc.namespace_package.is_package_or_sage_namespace_package_dir`, which we will also use in #28925.
+We implement a new function `sage.misc.package_dir.is_package_or_sage_namespace_package_dir`, which we will also use in #28925.
This change also removes a runtime dependency of the doctester on Cython, see #33029.
Ready for review
Branch pushed to git repo; I updated commit sha1. New commits:
3939af7 | Merge tag '9.5.rc3' into t/33033/prepare_sage_doctest_for_namespace_packages |
Branch pushed to git repo; I updated commit sha1. New commits:
cf40d61 | Merge tag '9.5' into t/33033/prepare_sage_doctest_for_namespace_packages |
Branch pushed to git repo; I updated commit sha1. New commits:
5adae47 | src/sage/misc/package_dir.py: Fix pycodestyle warning |
LGTM.
Changed reviewer from Tobias Diez, ... to Tobias Diez, Kwankyu Lee
Thank you!
Changed branch from u/mkoeppe/prepare_sage_doctest_for_namespace_packages to 5adae47