Repository navigation
Create a public API - #194
Conversation
818b425 to
21102fb
Compare
yguclu
left a comment
There was a problem hiding this comment.
Great job!
My single big concern is using the new public interface sympde.api in the unit tests in the tests folders. I have three main reasons to believe we should not do it:
- The unit tests are not "public-facing", as they only exist to ensure that every library component works as intended.
- The unit tests should know exactly the SymPDE modules they import from, and show them to the developers.
- As things progress, we hope that our unit tests will approach 100% coverage of the library. This will make
sympde/api/__init__.pya huge file which is not easily maintained.
If we avoid using sympde.api in the unit tests, we could restrict its imports to those needed by the projects that we know for sure to depend on SymPDE. AFAIK those are just PSYDAC, IGA-Python, and GeLaTo. This will make the file much shorter, cleaner, and easier to maintain.
Mention the PR number in its entries. Since the removed re-exports raise an ImportError immediately instead of a warning, list them under "Removed" rather than "Deprecated". Also record the removal of sympde.old_sympy_utilities.
21102fb to
333a249
Compare
|
Thanks! I agree on all three points.
I also rebased this branch onto the current |
|
For me this PR is ready to be merged, but I will wait for #193 before approving it |
Mention the PR number in its entries. Since the removed re-exports raise an ImportError immediately instead of a warning, list them under "Removed" rather than "Deprecated". Also record the removal of sympde.old_sympy_utilities.
3df371e to
d29190d
Compare
Expose supported user-facing symbols from sympde.api without relying on package-level wildcard exports.
Remove package-level wildcard exports and obsolete SymPy compatibility helpers, migrate internal imports and tests to defining modules, and defer costly optional plotting and serialization imports.
Move maintained notebook material into tested documentation examples and remove obsolete tutorial helpers and notebook assets.
Describe flat public imports in the README and add the API reference and executable examples to the Sphinx navigation.
The unit tests are not user-facing code, so they should not depend on the public sympde.api namespace. Import every object from the module that defines it instead, which shows developers exactly which modules a test exercises and keeps sympde.api independent of test coverage.
Importing __version__ from the top level of a package is standard (numpy, sympy), so sympde/__init__.py imports it again. It only loads sympde.version, so importing sympde stays lightweight. The documentation configuration imports the version from the top level again, and the initializer test allows this one import.
Explain how essential boundary conditions are passed to find, replace the ambiguous "executable" by saying that the examples are complete scripts run by the documentation workflow, and write "linearization of nonlinear problems" instead of "nonlinear linearization".
Write the definition x + i*y = A * (x1 + i*x2)**2 in the docstring of the custom mapping example, together with the resulting coordinate expressions.
The helper was used only twice, so call integral(domain, ...) directly.
Rename the regression test and explain the bug it guards against: plot_domain fell back to a 3D identity mapping for unmapped 2D patches. Check that the boundary of the unit square is drawn, select the Agg backend through matplotlib so it also applies when pyplot is already imported, and skip the test if matplotlib is missing.
Mention the PR number in its entries. Since the removed re-exports raise an ImportError immediately instead of a warning, list them under "Removed" rather than "Deprecated". Also record the removal of sympde.old_sympy_utilities.
Keep only the names that PSYDAC, IGA-Python and GeLaTo import from SymPDE, plus those used by the SymPDE documentation, which reduces the namespace from 241 to 122 names. Add the 1D forms used by GeLaTo (Mass, Stiffness, Advection, AdvectionT, Bilaplacian, Basic1dForm), which were missing. Drop __version__, which is available from the top-level package again, and the unused ExteriorPullBack alias.
The images were generated by the removed doc/include/graph.py script and are no longer referenced by any page.
Use "Linearization of nonlinear problems", as in the quickstart guide.
Add an example that writes the expression trees of an operator and of an expanded derivative with SymPy's dotprint, and draw them in the quickstart guide with sphinx.ext.graphviz. The graphs are generated from the example at build time, so they cannot get out of date like the removed pre-rendered images. Install Graphviz in the documentation workflow and mention it in the README.
grad, div and dot were imported twice from sympde.calculus.core. Import curl together with the other operators instead.
The examples imported from the package initializers, which no longer re-export objects. Import from sympde.api instead, and Dot, which is not part of it, from sympde.calculus.core.
sympde.topology.Domain and sympde.topology.InteriorDomain no longer exist, since the package initializers are empty.
yguclu
left a comment
There was a problem hiding this comment.
I have just had another look at the code in this branch. Although not introduced here, there remain several module-level names starting with an underscore, which by Python convention marks them as private to their module, although they are used as public API. Since this PR defines what is public, could you sort them out here?
-
Used by PSYDAC: remove the underscore and add them to
sympde.api:_partial_derivatives→partial_derivatives(sympde.topology.derivatives)_logical_partial_derivatives→logical_partial_derivatives(sympde.topology.derivatives)_split_test_function→split_test_function(sympde.expr.evaluation)
-
Used by other SymPDE modules only: remove the underscore, but do not add them to
sympde.api:_coeffs_registery→coefficient_types(sympde.core.basic; it is a tuple of types, not a registry)_diff_ops→diff_opsand_generic_ops→generic_ops(sympde.calculus.core)_sanitize_arguments→sanitize_arguments(sympde.expr.basic)_get_domain→get_domain(sympde.expr.expr)
-
Used only inside their module: keep the underscore, but remove them from
__all__insympde.expr.evaluation, since__all__lists the public names of a module:_get_trials_tests,_replace_atomic_expr,_split_expr_over_interface,_tensorize_atomic_expr,_to_matrix_form,_unpack_functions. -
Unused:
_is_op_test_functioninsympde.calculus.coreis never used (sympde/exterior/compiler.pydefines its own copy), so it can be deleted together with its__all__entry.
Remove the leading underscore from the module-level names that other modules or downstream projects use: - partial_derivatives, logical_partial_derivatives and split_test_function, used by PSYDAC, are also added to sympde.api. - _coeffs_registery becomes coefficient_types, since it is a tuple of types, not a registry. - diff_ops, generic_ops, sanitize_arguments and get_domain are used by other SymPDE modules only, so they are not added to sympde.api. Remove the names that are private to sympde.expr.evaluation from its __all__, and delete the unused _is_op_test_function from sympde.calculus.core (sympde/exterior/compiler.py defines its own copy).
|
Thanks, done in d2ec86d. |
yguclu
left a comment
There was a problem hiding this comment.
Thanks @FrederikSchnack, this PR looks excellent now!
A couple of things should probably be changed in the CHANGELOG, though
Co-authored-by: Yaman Güçlü <yaman.guclu@gmail.com>
Co-authored-by: Yaman Güçlü <yaman.guclu@gmail.com>
Co-authored-by: Yaman Güçlü <yaman.guclu@gmail.com>
Co-authored-by: Yaman Güçlü <yaman.guclu@gmail.com>
Pass the DOT text of the expression-tree example directly to Graphviz through a small `expression-tree` directive, instead of writing it to doc/_generated and reading it back. The documentation build no longer creates files in the source tree besides the API pages, so the doc/_generated entry is removed from .gitignore.
945eb9a to
309b06e
Compare
Adapt the multipatch changes to the conventions introduced by #193 and #194: - Import objects from their defining modules in the tests and in multipatch_gallery.py, since the package initializers are now empty. - Empty sympde/utilities/__init__.py again, and export plot_domain in sympde.api from sympde.utilities.plotting. - Restore the version of sympde/utilities/utils.py from this branch, which only contains lambdify_sympde: the merge had brought back the old plotting functions. - Allow passing mapping and logical_domain of an Interface positionally, so that it can be reconstructed from its args. - Use orientation instead of ornt in test_structural_arguments_are_sympy_objects.
Import from sympde.api in the examples of Interface and Domain.join, and refer to Domain by its defining module in the plotting docstrings, since the package initializers are empty after #194.
Summary
Introduce a single, explicit public API for SymPDE and reduce the amount of code and third-party functionality loaded during package imports (fixes #182).
User-facing code should now import supported objects from
sympde.api:Internal SymPDE modules continue to import objects directly from their defining modules.
Public API changes
1. Add an explicit
sympde.apinamespaceA new
sympde.apipackage provides a documented, flat namespace containing the supported user-facing objects.The API contains the objects used by the projects that depend on SymPDE (PSYDAC, IGA-Python and GeLaTo) and by the SymPDE documentation, 125 names in total. They were found by collecting all SymPDE imports in these repositories and resolving them to their defining objects. Objects that are only used inside SymPDE or by its unit tests are not part of the API, which keeps
sympde/api/__init__.pyshort and easy to maintain.The API covers:
The namespace defines an explicit
__all__, making it possible to identify the supported API without relying on implementation details or wildcard imports.Tests verify that:
__all__exists;__all__does not contain duplicates;2. Remove wildcard exports from package initializers (fixes #181)
The previous package initializers recursively imported and re-exported entire subpackages:
Importing a small part of SymPDE could therefore load most of the library, including modules unrelated to the requested functionality.
The initializers for the following packages are now empty:
The top-level
sympde/__init__.pyonly containsfrom .version import __version__, sofrom sympde import __version__keeps working, as is standard for Python packages. It only loadssympde.version.This makes imports more predictable and prevents package initialization from eagerly traversing the full dependency graph.
3. Establish separate user-facing and internal import conventions
User code should use the convenient public namespace:
Internal library code and the unit tests import directly from the module that defines each object:
This separation provides a stable user-facing API while keeping internal dependencies explicit and avoiding circular imports.
Import-performance improvements
4. Avoid loading the complete library for package imports
Previously, imports such as:
executed wildcard imports from multiple package initializers. This loaded many unrelated symbolic, plotting, mapping, and expression modules.
With empty package initializers, importing a module now loads only its actual dependencies. The complete user-facing namespace is loaded only when explicitly requested through:
5. Import third-party libraries only where needed (fixes #180)
Some modules imported slow third-party libraries at module level, although only a few of their functions use them:
sympde.utilities.utilsimportedmatplotlib.pyplotandfrom mpl_toolkits.mplot3d import *, which are only needed byplot_2dandplot_3d;sympde.topology.domainimportedh5py,yamlandnumpy, which are only needed byDomain.exportandDomain.from_file.These libraries are now imported inside the functions that use them, and the wildcard import is removed. Importing SymPDE, or creating and manipulating symbolic domains, therefore no longer initializes Matplotlib, HDF5 or YAML.
6. Remove unused imports from the library modules
A large number of unused imports were removed from the core, topology, exterior-calculus, expression, and printing modules.
This includes unused:
Besides reducing import work, this makes each module’s dependency requirements easier to understand.
7. Break circular imports with local imports
The topology derivative module previously imported mapping classes at module scope, while the mapping module also depended on derivative definitions.
MappingandJacobianare now imported locally only in the logical-derivative path that needs them.This removes the module-level cycle without changing the public behavior of differential operators.
Compatibility and cleanup
8. Remove the obsolete SymPy compatibility module
The previous PR replaced usages of the local compatibility helpers with maintained SymPy utilities and native Python metaclass syntax.
Once the package-level wildcard exports are removed, the remaining compatibility module is no longer part of the import surface and can be deleted.
This removes:
9. Make module-level names used outside their module public
Several module-level names started with an underscore, which marks them as private to their module, although they were used by other SymPDE modules or by PSYDAC. Their underscore is removed:
_partial_derivatives,_logical_partial_derivatives(sympde.topology.derivatives) and_split_test_function(sympde.expr.evaluation) are used by PSYDAC, and are added tosympde.api;_diff_ops,_generic_ops(sympde.calculus.core),_sanitize_arguments(sympde.expr.basic) and_get_domain(sympde.expr.expr) are used by other SymPDE modules only;_coeffs_registery(sympde.core.basic) is renamed tocoefficient_types, since it is a tuple of types.The
__all__of every module now lists only public names, and the unused_is_op_test_functionis removed fromsympde.calculus.core.10. Import from defining modules in the tests
The unit tests no longer rely on wildcard package exports. Since they are not user-facing, they import every object from the module that defines it, which shows developers exactly which modules a test exercises and keeps
sympde.apiindependent of test coverage.Dedicated API tests verify:
The API checks use direct imports and assertions rather than dynamically executing generated import statements.
11. Fix plotting of unmapped two-dimensional domains
While adding coverage for deferred plotting imports, the unmapped 2D plotting path was found to create a three-dimensional identity mapping.
It now creates the correct two-dimensional identity mapping:
A regression test plots an unmapped square with a non-interactive Matplotlib backend and checks that the outline of the unit square is drawn.
Documentation changes
12. Document the supported public API
The README and Sphinx documentation now direct users to
sympde.api.A dedicated API page documents the explicit namespace:
The API documentation avoids generating duplicate cross-reference targets for objects that are also documented in their defining modules.
13. Replace obsolete notebooks with executable examples
The old notebooks and their Binder integration had become outdated and were not validated by CI.
They are replaced by focused Python examples covering:
All examples use the new public API, and so do the docstring examples.
14. Execute documentation examples in CI
The documentation workflow now runs every Python example before building the Sphinx pages:
This ensures that documented imports and examples remain executable as the API evolves.
Obsolete documentation helper scripts, notebooks, notebook styles, and the Binder badge are removed.
15. Draw expression trees in the documentation
The quickstart guide shows the expression trees of two symbolic expressions. A new example script writes them in the DOT language with SymPy's
dotprint, andsphinx.ext.graphvizdraws them. The graphs are generated from the code at build time, so they always match it. The documentation workflow installs Graphviz for this.This replaces
doc/include/graph.py, which no longer ran, and its pre-rendered images, which are removed.Breaking changes
Wildcard convenience imports from package namespaces are no longer supported.
For example, user code should migrate from:
to:
Code that intentionally depends on internal implementation details may instead import directly from the defining module:
The
sympde.apinamespace is the recommended interface for downstream users.The
sympde.old_sympy_utilitiesmodule is removed. Usesympy.utilities.iterables.is_sequenceand Python'smetaclasskeyword instead of itsis_sequenceandwith_metaclass.The module-level names
_partial_derivatives,_logical_partial_derivativesand_split_test_functionlost their leading underscore. Import them aspartial_derivatives,logical_partial_derivativesandsplit_test_functionfromsympde.api.Validation
The complete test suite passes:
The documentation validation also passes: