Refactor packaging

Hi all,

This MR aims to improve the packaging of the toolbox. In general, deleting globals tend to have unwanted consequences. For instance, I have encountered import problems during unit testing. The refactoring has a similar outcome as the previous implementation, which is clean namespace. This is due to the __all__ being listed to each modules, as suggested here.

I also have listed all the required libraries for the Toolbox. One can now freely install the library on a local environment with pip install -e ".[maxwell]". The maxwell tag is a side effect of a direction that I would like to suggest. That is, the Toolbox codes can be used elsewhere; maybe in conjunction with another library, maybe as a Karabo device.

Please feel free to let me know your thoughts :)

@lleguy @mercurio @mercadil @teichman

changed the description

Indeed, importing everything and deleting parts of it is just awful.

But I actually do not like star-imports either. It makes finding something really hard: you imported a function from somewhere, but there it is not even mentioned, because it is a star-import from somewhere else.

Star-importing is fine on the command line or in a notebook, but should not be used in libraries.

Making the toolbox a better package is a gread idea.

You got me here XD I was thinking about the best way to structure the __init__ files, and I opt with using star imports as it reduces the steps needed to integrate a new function.

For instance, when one would add a new function in the routines module (say, toolbox_scs.routines.new_routine:new_function).. with star imports, he could easily integrate the new_function by:

1. adding an __all__ = ['new_function'] on routines/new_routine.py
2. adding a + new_routine.__all__ to routines/__init__.py.

Then, the new_function can now be used directly with tb.new_function().

If you suggest that tb.routines.new_routine.new_function() should work also, I could add the module names on the __all__. I have seen that it's being used by numpy in such manner also.

mentioned in merge request !108 (merged)

I would have considered it not such a big deal that a new function has to be listed in one more file, but I can also see your point. Just merge it and if it should ever become a problem we can still fix it.

In short, LTGM.

added 11 commits

• efd3ff18...4c1e5e5f - 2 commits from branch master
• 7218b61e - Refactor detectors module importing
• 4889de70 - Refactor misc module importing
• 739ac0c2 - Refactor routines module importing
• 9a8291a1 - Refactor top-level module importing
• 67b84d7f - Use absolute import on module alias
• 4e1b1c1e - List complete required package
• f3a52c4a - Use extra-requires to sort libraries according to usage
• b86aaffa - Update docs and remove some detector modules from init
• b480b8f1 - Add PES detector

Compare with previous version

assigned to @lleguy

Thanks for the review!

In principle, this is ready to be merged. Would be glad to have further looks as well :D

mentioned in merge request !107 (merged)

merged

mentioned in commit 01fe436c