You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Aiming for simulations results only, you don't think about pretty code.
It is likely to result in discussions which are essentially about taste.
There are multiple solutions for particular issues where it is not clear which one is optimal.
A crowded preamble in the module with all kinds of imports can negatively impact the total import time of PorePy and turn refactoring into a real pain, but it can make the code contained more readable.
I want to give some technical input because due to PorePy's complexity I regularly struggle with circular imports and the question "will this code be used anywhere else". It's not about "where to put what" or "this should not be in this namespace".
TLDR; ---
Type annotation always via pp.<namespace>.* and use from __future__ import annotations. If not possible, use typing.TYPE_CHECKING to avoid actual import.
Every subpackage should be a member of pp. Every object inside a subpackage used outside its own module should be a member of pp.subpackage. (keyword: use __all__ in modules to define what should be part of the subpackage).
Use the public API and not direct imports. I.e., give preference to usage of type pp.<namespace>.Foo to avoid crowded preambles and a burdening of the import procedure in terms of time and circularity. Direct imports execute the import of the respective modules again. Use this approach especially in scoped usages!
Consider namespace promotions (pp.ad.Operator to pp.Operator) only for objects used frequently in other subpackages. Formalizing what is "frequent" is to some degree a matter of taste, but often clear from a mathematical-model-perspective.
---
Let's consider two cases:
Active usage of an object in other modules (calls, inheritance,...)
Usage purely for typing (hints, linters, mypy,...)
We start with some object pp.<namespace>.Foo.
Typing usage
Typing must not in any case impede the import of PorePy or make it more prone to things like circular import issues.
But thankfully, that is very easily to fix:
from __future__ import annotations
from typing import TYPE_CHECKING
Case 1: Foo is important enough to be among PorePy's members
__future__.annotations will take care of any usage of Foo in typing, even if it is technically not yet imported into pp.
from __future__ importannotationsimportporepyasppdefbar(foo: pp.<namespace>.Foo) ->None:
...
Case 2: Foo is not among PorePy's members.
If its importance grows, it will be added at some point. But until then:
from __future__ importannotationsfromtypingimportTYPE_CHECKINGimportporepyasppfromporepy.<namespace>importFoo# DO NOTifTYPE_CHECKING:
fromporepy.<namespace>importFoo# DOdefbar(foo: Foo) ->None:
...
In the negative example above we see where the proposed solution was implemented for obj. but not for discr.
We also see that we have two classes called Discretization. But that is topic for some other discussion.
Active usage:
There are two things to consider:
Is it imported in an module within the same subpackage, or is it imported in another subpackage at all.
Is it used at import time (e.g., inheritance Bar(Foo)) or is it used within a scope (e.g., an instance of Foo is created inside some method)
And this is where it gets messy:
Case 1: Module in same subpackage, Usage at import time Case 2: Module in same subpackage, Usage within scope Case 3: Module in other subpackage, Usage at import time Case 4: Module in other subpackage, Usage within scope
Intuitively, subpackages can be expected to be part of the top package.
Like ad, models or grids could be part of pp, i.e., pp.ad, which is fine for most parts.
So by this intuition, Foo should be available as pp.<namespace>.Foo if it is important enough to be used outside its own module.
And the more critical/frequently used it is, the higher I would expect it in the namespace (pp.Foo, like pp.Grid, matter of taste!).
If so, case 4 is trivial: Don't import Foo directly, use pp.<namespace>.Foo if the usage is within a scope and in another subpackage. Having no direct import here reduces stress on the overall import strategy and time.
Case 3 is trivial if the two subpackages have hierarchical functionality. Just import the subpackage of Fooglobally before any other subpackage which relies on Foo (in porepy.__init__). If the functionality is not completely hierarchical but to some degree complementary, direct imports of Foo are required, if Foo is not already available of pp.<namespace>.Foo.
Case 2 can be trivially solved, if an agreement is made that we prefer the usage of pp.<namespace>.Foo even within the same subpackage. This requires an agreement because "if the python files are next to each other" the modus operandi for the average developer is direct and even relative imports I would say. But this agreement would optimize the import to some degree and make it more robust.
Negative example: Equation system
importporepyasppfrom .operatorsimportMixedDimensionalVariable# REMOVEclassEquationSystem:
defmd_variable(...):
returnMixedDimensionalVariable(...) # DO NOTreturnpp.ad.MixedDimensionalVariable(...) # DO
In any case, both objects are available as pp.ad.MixedDimensionalVariable and pp.ad.EquationSystem.
The EquationSystem one could also expect at the toplevel as pp.EquationSystem.
Case 1 is the trickiest because the functionality of a subpackage is quite interdependent. Though I would like to point out that there is always a local hierarchy reflecting the purpose of the subpackage.
Example: pp.ad.
Everything in ad is built around ad.operators, so that is the most important module. Modules like forward_mode work technically as stand-alone modules, so they need no imports at all, except for typing. Modules like ad_utils can technically be imported as well without importing anything else (again, except typing). So ad.operators can import anything directly from ad.forward_mode and ad.ad_utils directly, but not vice versa.
Negative example: ad_utils
from . importoperators# PRONE TO CIRCULAR IMPORTclassMergedOperator(operators.Operator):
...
Unresolved points:
Direct imports do shorten the code and make it more readable, and are as mentioned the preferred modus operandi.
PorePy needs a global import order motivated by mathematical modelling steps, but I think that is largely covered.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
This is a tricky issue, for a few reasons:
I want to give some technical input because due to PorePy's complexity I regularly struggle with circular imports and the question "will this code be used anywhere else". It's not about "where to put what" or "this should not be in this namespace".
TLDR;
---
pp.<namespace>.*and usefrom __future__ import annotations. If not possible, usetyping.TYPE_CHECKINGto avoid actual import.pp. Every object inside a subpackage used outside its own module should be a member ofpp.subpackage. (keyword: use__all__in modules to define what should be part of the subpackage).pp.<namespace>.Footo avoid crowded preambles and a burdening of the import procedure in terms of time and circularity. Direct imports execute the import of the respective modules again. Use this approach especially in scoped usages!pp.ad.Operatortopp.Operator) only for objects used frequently in other subpackages. Formalizing what is "frequent" is to some degree a matter of taste, but often clear from a mathematical-model-perspective.---
Let's consider two cases:
We start with some object
pp.<namespace>.Foo.Typing usage
Typing must not in any case impede the import of PorePy or make it more prone to things like circular import issues.
But thankfully, that is very easily to fix:
from __future__ import annotationsfrom typing import TYPE_CHECKINGCase 1:
Foois important enough to be among PorePy's members__future__.annotationswill take care of any usage ofFooin typing, even if it is technically not yet imported intopp.Case 2:
Foois not among PorePy's members.If its importance grows, it will be added at some point. But until then:
Negative example:
ad_utilsIn the negative example above we see where the proposed solution was implemented for
obj. but not fordiscr.We also see that we have two classes called
Discretization. But that is topic for some other discussion.Active usage:
There are two things to consider:
Bar(Foo)) or is it used within a scope (e.g., an instance ofFoois created inside some method)And this is where it gets messy:
Case 1: Module in same subpackage, Usage at import time
Case 2: Module in same subpackage, Usage within scope
Case 3: Module in other subpackage, Usage at import time
Case 4: Module in other subpackage, Usage within scope
Intuitively, subpackages can be expected to be part of the top package.
Like
ad,modelsorgridscould be part ofpp, i.e.,pp.ad, which is fine for most parts.So by this intuition,
Fooshould be available aspp.<namespace>.Fooif it is important enough to be used outside its own module.And the more critical/frequently used it is, the higher I would expect it in the namespace (
pp.Foo, likepp.Grid, matter of taste!).If so, case 4 is trivial: Don't import
Foodirectly, usepp.<namespace>.Fooif the usage is within a scope and in another subpackage. Having no direct import here reduces stress on the overall import strategy and time.Case 3 is trivial if the two subpackages have hierarchical functionality. Just import the subpackage of
Fooglobally before any other subpackage which relies onFoo(inporepy.__init__). If the functionality is not completely hierarchical but to some degree complementary, direct imports ofFooare required, ifFoois not already available ofpp.<namespace>.Foo.In other words: DO IF POSSIBLE
OTHERWISE
Case 2 can be trivially solved, if an agreement is made that we prefer the usage of
pp.<namespace>.Fooeven within the same subpackage. This requires an agreement because "if the python files are next to each other" the modus operandi for the average developer is direct and even relative imports I would say. But this agreement would optimize the import to some degree and make it more robust.Negative example: Equation system
In any case, both objects are available as
pp.ad.MixedDimensionalVariableandpp.ad.EquationSystem.The
EquationSystemone could also expect at the toplevel aspp.EquationSystem.Case 1 is the trickiest because the functionality of a subpackage is quite interdependent. Though I would like to point out that there is always a local hierarchy reflecting the purpose of the subpackage.
Example:
pp.ad.Everything in
adis built aroundad.operators, so that is the most important module. Modules likeforward_modework technically as stand-alone modules, so they need no imports at all, except for typing. Modules likead_utilscan technically be imported as well without importing anything else (again, except typing). Soad.operatorscan import anything directly fromad.forward_modeandad.ad_utilsdirectly, but not vice versa.Negative example:
ad_utilsUnresolved points:
Beta Was this translation helpful? Give feedback.
All reactions