tobac icon indicating copy to clipboard operation
tobac copied to clipboard

Split `utils` module into internal utilities and external utilities

Open freemansw1 opened this issue 2 years ago • 5 comments

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

freemansw1 avatar Apr 17 '22 05:04 freemansw1

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.

JuliaKukulies avatar Apr 18 '22 15:04 JuliaKukulies

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 avatar Apr 21 '22 22:04 freemansw1

@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.

JuliaKukulies avatar May 03 '22 13:05 JuliaKukulies

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.

freemansw1 avatar May 03 '22 15:05 freemansw1

Ok, sounds good!

JuliaKukulies avatar May 04 '22 00:05 JuliaKukulies

Resolved with #191

freemansw1 avatar Nov 30 '22 15:11 freemansw1