Move get_auto_lims function to utils

[FrancescoNegri]

get_auto_lims computes the boundaries of the given probe. It relies exclusively on vanilla Python and numpy. The plotting namespace, where the function is defined, imports matplotlib, that might not always be available. Hence, get_auto_lims should be independent on matplotlib and my suggestion is to move it to utils.

[FrancescoNegri]

Should I write some documentation for get_auto_lims? It is currently undocumented.

[JoeZiminski]

Should I write some documentation for get_auto_lims? It is currently undocumented.

Yes please do, that would be brilliant, thanks!!

This sounds like a good idea, although I don't know this area of the codebase well. My only question is whether get_auto_lims will ever be needed outside of the context of plotting a probe? If not, then I think it makes sense to stay in plotting but if it is likely to be called in other contexts it should be moved.

[FrancescoNegri]

I don't know if it will be used outside the context of plotting, but it should be at least agnostic with respect to the plotting backend. If, for instance, I want to plot something with pyqtgraph or anything else I should be able to automatically compute the probe limits without having matplotlib installed.
The idea to move get_auto_lims to utils comes from here.

[JoeZiminski]

Nice! Okay that makes sense, if it is more related to plotting I would consider a plotting_utils.py module to stop general utils getting too busy, but I don't know this codebase well and if utils is small it might as well go there. I'll leave it up to yourself and @alejoe91 to decide, cheers!

[FrancescoNegri]

I was thinking that maybe the best thing is to add the get_auto_lims function directly to the Probe and ProbeGroup classes, since the probe spatial dimension is somehow a property of the class. Does this solution make more sense to you? :)

[FrancescoNegri]

I was thinking that maybe the best thing is to add the get_auto_lims function directly to the Probe and ProbeGroup classes, since the probe spatial dimension is somehow a property of the class. Does this solution make more sense to you? :)

@JoeZiminski @alejoe91 any thoughts about this? :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.92%. Comparing base (274b570) to head (de8a2cf).
Report is 56 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #281   +/-   ##
=======================================
  Coverage   88.91%   88.92%           
=======================================
  Files          10       10           
  Lines        1868     1869    +1     
=======================================
+ Hits         1661     1662    +1     
  Misses        207      207           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JoeZiminski]

Hey @FrancescoNegri sorry for the delay here, I think Alessio was happy with the location on utils and I don't really know anything about this codebase so sounds good to me! If you still wanted to add the docstring for get_auto_lims() in another PR feel free to open one. I'd be happy to review SI #3142 when useful now this has been resolved.

[FrancescoNegri]

Sounds great, thanks @JoeZiminski! I have been quite busy lately, but I will try and open a new pull request for the get_auto_lims() docstring soon.