Switch segmentation to use xarray internally

[freemansw1]

In line with #354, this PR switches segmentation to use xarray internally and also does some light refactoring of the segmentation package.

This PR includes #354 so that I could make sure that it all worked together. #354 should be merged first, so I am marking this as a draft PR until that is merged.

• Have you followed our guidelines in CONTRIBUTING.md?
• Have you self-reviewed your code and corrected any misspellings?
• Have you written documentation that is easy to understand?
• Have you written descriptive commit messages?
• Have you added NumPy docstrings for newly added functions?
• Have you formatted your code using black?
• If you have introduced a new functionality, have you added adequate unit tests?
• Have all tests passed in your local clone?
• If you have introduced a new functionality, have you added an example notebook?
• Have you kept your pull request small and limited so that it is easy to review?
• Have the newest changes from this branch been merged?

[github-actions]

Linting results by Pylint:

Your code has been rated at 8.72/10 (previous run: 8.72/10, +0.00)
_{The linting score is an indicator that reflects how well your code version follows Pylint’s coding standards and quality metrics with respect to the RC_v1.6.0 branch.
A decrease usually indicates your new code does not fully meet style guidelines or has potential errors.}

[freemansw1]

The codecov results indicate to me that we will need to specifically exclude the versions of xarray/iris that cause a conversion incompatibility when we release 1.6.0.

[w-k-jones]

Awesome work, let me know if you would like me to start reviewing :)

[freemansw1]

Note: 360 day calendar issues and times not matching up between feature detection and segmentation

[JuliaKukulies]

@freemansw1, was this already ready for a re-review (maybe after #354 is merged) or not quite yet?

I can have a look at what is going on with the Jupyter CI since it seems to be caused by the bulk statistics. For some reason, the iris_to_xarray decorator breaks here when get_statistics_from_mask() is called:

tobac/tobac/utils/bulk_statistics.py

Lines 214 to 217 in 201c601

	@decorators.iris_to_xarray()
	def get_statistics_from_mask(
	features: pd.DataFrame,
	segmentation_mask: xr.DataArray,

tobac/tobac/utils/decorators.py

Line 178 in 201c601

output = func(*args, **kwargs_new)

We do not use the decorator when the bulk statistics are computed within the feature detection/ segmentation, so the offline computation is the only notebook that fails. It is odd, though, because the input format looks the same as in the working version and the unit test for that function also passes. I will look more into this.

[freemansw1]

Thanks, @JuliaKukulies . I'm hesitant to mark this ready for review until #354 is merged, but happy for you to look into the bulk statistics issue.

[freemansw1]

OK, I'm working on more thoroughly testing this now that #354 is merged. One issue that I'm consistently noticing is issues between nanosecond/microsecond conversions. This is a note to myself to add the ability to find segmentation that is padded or some other approach to this.

[JuliaKukulies]

@freemansw1 do you know what is going on with the matrix test fail here? I get the same for #448 and #449 (although the local tests all pass). It would be nice to solve this to prepare so we can get ready for the release by the end of this week maybe

[w-k-jones]

@freemansw1 do you know what is going on with the matrix test fail here? I get the same for #448 and #449 (although the local tests all pass). It would be nice to solve this to prepare so we can get ready for the release by the end of this week maybe

I noticed the same issue a few weeks back in #444. It's due to a depreciated feature in the newest numpy version which affects transform feature points if no points are actually removed. I'll try and either create a PR to fix it this week, or provide more details if someone else has time to fix it first

[JuliaKukulies]

@freemansw1 do you know what is going on with the matrix test fail here? I get the same for #448 and #449 (although the local tests all pass). It would be nice to solve this to prepare so we can get ready for the release by the end of this week maybe

I noticed the same issue a few weeks back in #444. It's due to a depreciated feature in the newest numpy version which affects transform feature points if no points are actually removed. I'll try and either create a PR to fix it this week, or provide more details if someone else has time to fix it first

Thank you @w-k-jones! If you have already looked at this and an idea how to fix it, this would be great. If you do not have any time for it, maybe you can create an issue and describe what needs to be fixed so I could go from there.

[freemansw1]

Noting potential incompatibilities here:

• May need to update xarray requirement to newer than 2023.9.0 due to using casting in field.astype (seems that this may be a windows-specific issue)

[codecov]

Codecov Report

Attention: Patch coverage is 44.63453% with 356 lines in your changes missing coverage. Please review.

Project coverage is 59.37%. Comparing base (d6469b1) to head (82af748).
Report is 117 commits behind head on RC_v1.6.0.

Files with missing lines	Patch %	Lines
tobac/utils/internal/general_internal.py	0.00%	186 Missing ⚠️
tobac/analysis/cell_analysis.py	12.19%	144 Missing ⚠️
tobac/analysis/feature_analysis.py	22.58%	24 Missing ⚠️
tobac/segmentation/watershed_segmentation.py	97.72%	1 Missing ⚠️
tobac/wrapper.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##           RC_v1.6.0     #417      +/-   ##
=============================================
+ Coverage      58.74%   59.37%   +0.62%     
=============================================
  Files             20       25       +5     
  Lines           3629     3884     +255     
=============================================
+ Hits            2132     2306     +174     
- Misses          1497     1578      +81     

Flag	Coverage Δ
unittests	59.37% <44.63%> (+0.62%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[w-k-jones]

Very nice all around. My comments are all fairly minor, and I'd like to hear your views on whether they need addressing

[freemansw1]

Thanks, @w-k-jones. Agreed with all of your comments.

[w-k-jones]

Thanks for the changes. I assume that the remaining failing tests will be resolved after merging the fixes in the RC_v1.6.0 branch. Happy for this to be merged