[patch] use deprecate from pyiron_snippets #1432
Conversation
| ) | ||
| from pyiron_snippets.deprecate import deprecate |
There was a problem hiding this comment.
The import feels a bit redundant from pyiron_snippets.deprecate import deprecate. Can we change it to from pyiron_snippets import deprecate ?
There was a problem hiding this comment.
Certainly not on this version of snippets. Personally, to maintain the idea that snippets is exceedingly lightweight and that all its pieces are independent, I prefer to leave its __init__ empty. But I'm not totally closed to it so feel free to open an issue on snippets and see what the others think
| from pyiron_snippets.deprecate import deprecate | ||
| from pyiron_snippets.import_alarm import ImportAlarm |
There was a problem hiding this comment.
Especially this line would be much cleaner as:
| from pyiron_snippets.deprecate import deprecate | |
| from pyiron_snippets.import_alarm import ImportAlarm | |
| from pyiron_snippets import deprecate, ImportAlarm |
There was a problem hiding this comment.
As mentioned above, I don't want this because if snippets grows I don't want to burn time importing everything in it just because you use one thing in it. But tbh I'm not sure that even with a large module this burns significant time, so if you have data to prove it won't impact performance down the line, then I'm open to it.
There was a problem hiding this comment.
The current way keeps the different submodules of snippets and their API uncoupled as is intended and so I think it is strongly preferable to importing/exporting everything in the top module. Especially since it would be for purely aesthetic reasons. from pyiron_snippets.deprecate import * would work just as well, if it's really unbearable.
i.e. pyiron_base.state.logger.logger with pyiron_snippets.logger.logger. The original location leaves a re-direct for backwards compatibility.
[patch] replace logger with snippets version
No description provided.