On Jun 29, 2010, at 9:41 AM, Benny Malengier wrote:
> First, I'd like to mention I'm happy with how fipy is handling the problem > I'm throwing at it. > I just write here to offer some insight from a user using fipy for the first > time. Thank you for sharing your experiences (and for submitting patches!). > I have struggled with a bug for a long time because it was unclear that a > Grid2D created from > from fipy import * > can be a Grid2D or a UniformGrid2D > > I think it would be a better design if Grid2D in meshes.numMesh is renamed > NonUniformGrid2D. > > I also find it impossible to write generic code that can work on all meshes. > UniformGrid2D does not call the __init__ method of it's parent class. I find > that a dangerous way of writing inheritance. Eg, for UniformGrid2D, > self.areaProjections attribute is not present, altough it is an attribute of > the parent object (it is set in __init__ of /meshes/common/mesh.py). The UniformGrid?D.areaProjections attribute does not (and will not) exist. The purpose of the UniformGrid?D classes is that they don't store anything. The UniformGrid?D._getAreaProjections() method *does* exist, though, and is what should be called if you need these values. We recognize that the modern solution to this is to have a UniformGrid?.areaProjections @property, but that was not available when this part of FiPy was written and other tasks have take precedence over refactoring this. Pervasive introduction of properties is planned, though. > but it would be nice if this is not needed, eg by adding the __init__ or by > making in the manual mush clearer that Grid2D is not the nummesh/Grid2D. Renaming fipy.meshes.numMesh.grid?D.Grid?D to fipy.meshes.numMesh.nonUniformGrid?D.NonUniformGrid?D and then introducing a common base class for NonUniformGrid?D and UniformGrid?D almost certainly can and should be done. Likewise, we can certainly clarify the documentation for the Grid?D() factory functions. The current arrangement is an artifact of history; Grid?D existed first and was not changed when we introduced UniformGrid?D for memory efficiency. I can see that the current names can be confusing. We are not likely to add a call to the base class's __init__() as that would defeat the point of the UniformGrid?D classes (i.e., that they store virtually nothing and calculate what they need on the fly). While invoking base's __init__() is typical, I am not aware that it is a requirement so long as the subclass implements the entire published interface of its base classes (and neither areaProjections or _getAreaProjections() are intended to be part of the published interface). If you know of any pronouncements from BDFL on the matter, I'd be happy to see them. A complete refactoring of the mesh classes is probably warranted, but there are some conflicting inheritance relationships that so far elude a clearer arrangement. Proposals welcome.
