artofscience / SAOR

Sequential Approximate Optimization Repository
GNU General Public License v3.0
5 stars 1 forks source link

Best practice for importing classes #76

Closed Giannis1993 closed 2 years ago

Giannis1993 commented 2 years ago

Right now we have:

If the package is imported as import sao, then these classes can be used as:

which is very nice. However, if a user writes from sao import *, then wouldn't the Mixed() have a double meaning?

My question is what is the best practice here? Should we refactor the above Mixed() classes to MixedML(MoveLimit) and MixedInt(Intervening) to avoid this, or is it better to leave them with the same name?

MaxvdKolk commented 2 years ago

The from sao import * is considered "bad practice" I think, for the reasons you mention. This will populate the namespace with everything, possibly having overlapping imports. Also, those linting tools typically do not like it, as it makes it harder to track if the imports are used or not etc.

I think it would be nice to layout the __init__.py files, such that we can do imports along the line of from sao.move_limits import Mixed etc. The overlapping could be resolved by the user as

from sao.move_limits import Mixed as MixedMovelimit
from sao.intervening_variables import Mixed as MixedInterveningVariables

However, it might make sense to rename these classes ourselves already. The Mixed name on its own is not that clear. It is clear when given as Mixed(Intervening), but that is not how a user would see it I think. Maybe we rename these classes to MixedMoveLimit and MixedIntervening? If that is too long, a user can define shorthands themselves

from sao.move_limits import MixedMoveLimit as MML
from sao.intervening_variables import MixedIntervening as MIV
etc
artofscience commented 2 years ago

IMO circumvent Import *

and rename eg mixed to mixedmovelimit would be perfect

Giannis1993 commented 2 years ago

Done