Module names overwritten by classes

[colleenjg]

I thought I'd mention this as a refactor is now in the works: it's not possible to import the modules, instead of the classes (e.g., Neurons.py instead of the Neurons class). This is due to the * imports in the __init__.py overwriting the module names in the namespace.

For example, on dev:

In [1]: import ratinabox
In [2]: import ratinabox.Neurons as imported_Neurons
In [3]: imported_Neurons
Out[3]: ratinabox.Neurons.Neurons

The same thing happens on v2_beta (tested after PR #58).

In [1]: import ratinabox
In [2]: import ratinabox.neuron.Neurons as imported_Neurons
In [3]: imported_Neurons
Out[3]: ratinabox.neuron.Neurons.Neurons

I haven't found a clean way around this to access the module. This is needed, for example, when you want to reimport a module that is in use, e.g.

In [1]: import ratinabox
In [2]: import ratinabox.neuron.Neurons as imported_Neurons
In [3]: import importlib
In [4]: importlib.reload(imported_Neurons) # this fails, as it is not a module

I don't know what the standard way is to address this. I think in some cases there is the use of underscores and/or lowercase, e.g., _neurons.py or neurons.py, or doing the * import at a different depth.

[TomGeorge1234]

Interesting. I see the problem. i could rename the module but I'd rather not. Seems worth fixing but I don't know the most standard way to fix this. Anyone?

[pierreglaser]

Workaround:

>>> import importlib
>>> importlib.import_module("ratinabox.Neurons")
<module 'ratinabox.Neurons' from '/Users/pierreglaser/.local/miniforge/envs/ratinabox/lib/python3.9/site-packages/ratinabox/Neurons.py'>

To avoid having to use importlib, consider removing star-imports from __init__.py.

[TomGeorge1234]

thanks @pierreglaser.

It's coming back to me now, I added star-imports because I briefly wanted things like

from ratinabox import *
Env = Environment()

but I never adopted this import style and don't think anyone else has either. I just removed them in latest push.

[colleenjg]

Thanks @pierreglaser for the workaround. I'll test it out.

Quick note: @TomGeorge1234, removing the * imports is likely to cause a big impact on everyone's repos, as it should break all imports of any Agent, Env or Neuron classes. Just want to check that that is the intention.

Clarification: I omitted to clarify that although the * imports cause the problem described above in the dev branch, in the v2_beta branch, from ratinabox.env.Environment import Environment in ratinabox/env/__init__.py, for example, has the same effect. It overwrites the module with the class, in the attributes of ratinabox.env.

I see a lot of packages that use this type of import in the inits to make imports less deep - I do think that's fine. Where it fails is where the class and file names also match exactly. The most standard fix is probably to rename the modules to lowercase, as mentioned above. This would also align the package with PEP 8 guidelines, where modules have short, all-lowercase names, with underscores if needed, and classes are named in CapWords, as you've done.

Is there a particular reason you don't want to rename the modules? Unless I've overlooked something, changing their names shouldn't break the code for most users, given that the current import convention has been overwriting the modules in the namespace anyway.

[colleenjg]

Clarification: The removal of the star imports will break current users' imports if they were using import ratinabox, but not if they were using, like you were, from ratinabox.Environment import Environment.

[colleenjg]

So, to be fair, the change of the module names would break imports for people who have been using from ratinabox.Environment import Environment. Which is why, if this name convention change were adopted, I do think it would be best as part of the overall refactor.

[TomGeorge1234]

Re quick note Yeah, it shouldn't break if people do

import ratinabox
from ratinabox.Environment import Environment
from ratinabox.Agent import Agent
from ratinabox.Neurons import PlaceCells, GridCells #...

which I assume is essentially everyone. But if you're okay with the workaround I'll put back the star import (just to be safe) and add it to the refactor down the line.

Re clarification Yeh so I definitely can't change the module names in RiaB1.x. For 2.0 I agree and will rename Environment.py ---> environment.py, PlaceCells.py ---> placecells.py etc. which should fix this issues and align with PEP8. Thanks for the heads up!

[colleenjg]

That's right - removing the star import would only break imports if they were doing from ratinabox import Environment, Agent, Neurons. I think since no one else appears to have encountered the issue I reported here, I agree - I think it's fine to keep the star import for now, and address this in the refactor!