search

kinnala / scikit-fem

Simple finite element assemblers
https://scikit-fem.readthedocs.io
BSD 3-Clause "New" or "Revised" License
504 stars 80 forks source link

Refactor some Mesh methods to utils #50

Closed kinnala closed 6 years ago

kinnala commented 6 years ago

I propose that in order to shorten the already massive mesh module, we move some methods to utils as separate functions. Candidates are:

gdmcbain commented 6 years ago

Would this compromise discoverability #3? On loading a Mesh, its methods can be discovered by introspection, e.g. help and dir.

gdmcbain commented 6 years ago

Alternative idea for shortening modules: split them into subpackages. Something like:

Another advantage of this file-for-each-class style is that the inheritance tree can easily be read from the directory tree.

gdmcbain commented 6 years ago

O. K. I'll start implementing this in a branch; see whether you like it. (There are many opinions around about this sort of thing and I don't mean to be prescriptive.)

https://github.com/gdmcbain/scikit-fem/compare/one-class-one-file

kinnala commented 6 years ago

So you suggest having functions in __init__.py and otherwise one class per file?

gdmcbain commented 6 years ago

Hmm, no, not necessarily. I realized later that it's a house-style that was imposed on me about a decade ago, so in part it's what I'm used to. I've been happy enough with it, and it would serve

in order to shorten the already massive mesh module

but I did see that there were a variety of opinions about this around the Internet. Possibly some of those belong to programmers with better understanding of such matters than me. Some say it's not Pythonic.

If adhered to strictly, it does either cause or reveal problems; e.g. in Mesh.from_meshio (recently Mesh.load) when it checks issubclass(cls, Mesh2D), that's a circular dependency between Mesh and Mesh2D.

I thought I'd just try it out in a branch on the fork to see what it looked like.

kinnala commented 6 years ago

Thanks for these ideas. I decided to switch to your proposed structure with some small modifications. I hope everything's is now correctly there. I'm probably going to do something similar with element module in the future.