tobac
tobac copied to clipboard
Split `utils` module into internal utilities and external utilities
We should split the utils.py
module into an internal utilities and external utilities module to save ourselves (and our users) some headaches down the road.
Proposal
Keep all external utilities in utils.py
to preserve backwards compatibility, move all internal utilities to something like internal_utils.py
.
Proposed division:
External Utilities
-
column_mask_from2D
-
mask_cube_cell
-
mask_cube_all
-
mask_cube_untracked
-
mask_cube
-
mask_cell
-
mask_cell_surface
-
mask_cell_columns
-
mask_cube_features
-
mask_features
-
mask_features_surface
-
mask_all_surface
-
get_spacings
Internal Utilities
-
add_coordinates
-
get_bounding_box
-
get_label_props_in_dict
-
get_indices_of_labels_from_reg_prop_dict
I think that this subdivision would make sense. Maybe more than the division into mask.py
and general.py
as in the subpackage utils
on v2.0-dev
? If the plan is to keep mask.py
and general.py
in v2.0-dev
, then it would maybe be better to split utils.py
in the same way it is done there. But I think your proposal sounds good and could be moved to v2.0-dev
later on as well.
Great point @JuliaKukulies! When dealing with this particular headache, I hadn't thought to look how v2.0-dev
handled things. I think we can actually go ahead and move utils
into its own submodule without breaking anyone's existing code and start to bring things closer to the way they should be in v2.0-dev
. I'll look into this and put in a PR whenever I get time.
@freemansw1 I wanted to start working on a PR to bring the spectral filtering into dev
so it will be ready for v1.5. Do you think I should wait until we have moved utils
into a submodule? In v2.0-dev
, I put spectral_filtering
in general.py
, but I am happy to move it somewhere else if you think it fits better in feature_detection.py
itself for example.
I am inclined, for now, to just keep everything in utils.py
? That's what we've done with the 3D/PBC work that should be in a PR today or tomorrow. After both of those are merged (and splits/mergers), we can separate things out.
Ok, sounds good!
Resolved with #191