pyxem
New styling of imports
I suggest a new "styling" of imports in the documentation (and test suite). Instead of importing functions and classes, we import modules.
Current:
from orix.quaternion import Rotation
Rotation.random()
New:
import orix.quaternion as oqu
oqu.Rotation.random()
If we're good at marking files as private (orix/quaternion/_misorientation.py etc.), users can look up of which functions and classes a module provides by tabbing with the cursor at oqu. etc.
Each module will then be importing accordingly:
import orix.data as oda
import orix.crystal_map as ocm
import orix.io as oio
import orix.measure as ome
import orix.plot as opl
import orix.projections as opr
import orix.quaternion as oqu
import orix.sampling as osa
import orix.vector as ove
I've look at this a few times and had mixed thoughts, but I think I'm on board now. I tried rewriting a few examples in this suggested style, most felt better, some felt more convoluted, but I think that's just because this is new.
Pros:
- Orix has some crazy long import sections. This idae would solve that.
- If you think of
from orix.quaternion import Rotationas akin tofrom scipy.linalg import eig, it feels almost irresponsible from a code comprehension standpoint that we import bare classes and functions the way we do. - We would never again have the frustrating experience of mixing up
scipy.spatial.transform.Rotationwithorix.quaternion.Rotationagian.
The only real addition I have is I think ultimately I would prefer something more like:
import orix as ox
import numpy as np
r =ox.Rotation.from_axes_angles(ox.Vector3d.zvector(), np.pi/4)
where the major classes are imported at the top-level, but the style change here wouldn't make top level imports more or less difficult to add later.
I'm for it, it will just take some time to get used to.
I'm curious what other people think though.
Thank you for the quick response, @argerlt!
it will just take some time to get used to.
Yes, it will. But, current imports will still work, so it's optional for the user.
I tried rewriting a few examples in this suggested style, most felt better, some felt more convoluted
Yeah, needing the three-letter prefix, e.g. oqu, is a bit more convoluted, but not much!
I would prefer something more like:
import orix as ox import numpy as np r =ox.Rotation.from_axes_angles(ox.Vector3d.zvector(), np.pi/4)
I definitely see the appeal. However, the general trend is for established Python libraries to push for module-level imports, not top-level ones. SciPy, Matplotlib, and scikit-image, for example. I suggest we stick to the same and encourage importing from modules.
@hakonanes I personally dont ever really import modules. I think it becomes difficult to track all of the module short names and it's not hard to just make a long list at the top.
I also agree with @argerlt, but to maybe put a finer point to his suggestion. My problem is the names of the modules.
Mostly the quaternion and crystal_map modules are confusing. A module name shouldn't mirror a class name as there are other classes in that module.
from orix.crystal_map import Phase confused me for a for a long while. And looking through the modules now I don't really have the first guess as to where the symmetries and point groups would be imported.
Quaternion -> transform Crystal_map-> ???
I think it becomes difficult to track all of the module short names and it's not hard to just make a long list at the top.
Note that it's optional, both are equally valid. What I'm suggesting is for us to encourage module-level imports in the documentation only. You can continue to write scripts with class-level imports, but in the orix documentation, we use module-level imports.
My problem is the names of the modules.
I agree with you, but I don't think it's worth it to move classes. I suggest to keep it as-is just because current users are used to it.
The module names are historical. When we introduced the crystal map, we first thought of that, and then that we needed a class for crystallographic phases (symmetry + unit cell). The class was placed in the crystal map module since it was to be used there. Then, of course, it became required for crystal directions (the Miller class), and suddenly it doesn't make much sense for the phase class to be in the crystal map module.
looking through the modules now I don't really have the first guess as to where the symmetries and point groups would be imported
The symmetry class was placed in the quaternion module because it subclasses the quaternion class. I would not do it like that if I were to write the class again, but it's not worth it to change that fact, I think.
Quaternion -> transform
That would be a better name, as all classes in that module (except the orientation region) describe transformations. However, I think it's not worth changing that, either.
I definitely see the appeal. However, the general trend is for established Python libraries to push for module-level imports, not top-level ones. SciPy, Matplotlib, and scikit-image, for example. I suggest we stick to the same and encourage importing from modules.
@hakonanes not to push back on this too much because I don't think it matters too much either way but I don't think import orix.crystal_map as ocm is really a common pattern. I checked some more of the documenation. With most examples scipy, skimage and astropy which all seem to import things on a class by class basis ASE as well.
Matplotlib tends to do things more at the modular level but I also don't think that almost everyone agrees that matplotlib is just weird because it started out trying to copy matlab. And even matplotlib doesn't import at the module level for the more object oriented/ pythonic code like collections ;)
I guess oddly I would almost prefer:
from orix import quaternion
rot = quaternion.Rotation()
As that helps give more information about where the rotation object comes from and is less confusing than oqu. The thing about the current module names is that it doesn't provide additional context.
from orix import crystal_map
phase = crystal_map.Phase()
Doesn't help me understand anything about the Phase object other than it is somehow related to the crystal_map. It actually makes me a bit more confused as I expect the Phase and the CrystalMap objects to be very similar same thing with ocm but now it's slightly more abstract.
It wouldn't be the worst thing to deprecate modules and it would be pretty easy to not break peoples code. It's probably a 2 year long process but if there was ever a 1.0.0 version it would be nice to do it before then.
@CSSFrancis, I agree that this is not a very important change. But I'm still convinced this is a good idea.
I think the biggest benefit is console look-up of the module. This tells the user what a module contains. Here's an example with VS Code with Microsoft's Python extension:
After we've cleaned up the namespace, the user can browse all public functionality of a module, along with the docstring. All without importing anything.
Another benefit is fewer and more readable imports. When I create a script with orix, I always have to go back to the top to add new imports as I go along. It is easier if I just import the quaternion and vector modules and can then look up what I need from them.
Looking at the scikit-image and ASE examples you link, I find importing a data module and a write function to be too general names and something that can easily be overwritten by mistake in a longer script. As @argerlt mentions above, if we use oqu.Rotation() instead of Rotation(), we can import from scipy.spatial.transform import Rotation without a namespace clash.
Browsing through some of Matplotlib's examples, a fairly common pattern for them is
import matplotlib.collections as mcollections
import matplotlib.patches as mpatches
My suggestion is basically the same thing. I'd argue it's only marginally easier to remember the meaning of mcollections than oqu: if you forget, you can run oqu? or just look at the imports at the top.