-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reorganisation of utils #996
Comments
Done. |
What's left?
No action at this time.ransac_ellipse_tools.py |
I should really look through some of the old But I'm not sure exactly when I'll have the time. |
@pc494 I don't know if we are going to get much farther with this. I'm not sure that the signals should be private. There are too many instances where we want to directly create a signal that I think they should be public. |
As in the functions in |
I completely forgot we have a utils/signals. Yes that Should be private. I was thinking you were trying to make the signals module private and I was a bit confused. |
So I got started on the utils folder in #994 and I think I've got a handle on what we have. Broadly
utils
modules fall into three categories.Internal use only
e.g.
_deprecated
,_slicer
Functionality that the devs need but that we either don't expect or don't want end users to make use of. My plan here is to simply be more explicit with these modules (i.e making the private modules, rather than just private functions). For example I don't think we want our users trying to use our cuda code themselves . So cuda_utils would be moved to a private module (with required deprecations etc).
Support for niche functionality
We originally adopted a utils-heavy pattern. Many classes have supporting functions (that generally get shoved in
map
) that have their own util folders. I would consider it better to just move such functionality directly into the module that defines the class. This (for example) is what I've done in #994 for the utils that support theReducedIntensity
classes.True, user-facing utils
An obvious example of this group is plotting_utils. We want users to use them, they are utils. I don't plan to change much about these except to rename modules to drop the trailing `util'. I think it's uncontentious to suggest that:
is weird when you could have
with a deprecation warning on the old imports.
The text was updated successfully, but these errors were encountered: