Add return_edges argument to DataSet.assign_resolution_bins()

[JBGreisman]

Fixes #184 by adding a return_edges argument to DataSet.assign_resolution_bins(). This PR adds that flexibility to the API, but otherwise does not change the default behavior of the function to preserve backwards compatibility.

[kmdalton]

I have suggested a slight refactor. I haven't gone over the tests very carefully yet. I want to wait for your feedback on my suggestion.

[JBGreisman]

I made the requested refactors to bin_by_percentile() and the corresponding changes to DataSet.assign_resolution_bins() to preserve the user-facing API

[codecov-commenter]

Codecov Report

Base: 98.37% // Head: 98.35% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (5af73b1) compared to base (01a3e6c).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #187      +/-   ##
==========================================
- Coverage   98.37%   98.35%   -0.03%     
==========================================
  Files          45       44       -1     
  Lines        1788     1764      -24     
==========================================
- Hits         1759     1735      -24     
  Misses         29       29              

Flag	Coverage Δ
unittests	98.35% <100.00%> (-0.03%)	⬇️

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

Impacted Files	Coverage Δ
reciprocalspaceship/dataset.py	98.26% <100.00%> (+0.04%)	⬆️
reciprocalspaceship/stats/completeness.py	100.00% <100.00%> (ø)
reciprocalspaceship/utils/binning.py	89.65% <100.00%> (-0.97%)	⬇️
reciprocalspaceship/__init__.py

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[JBGreisman]

@kmdalton I think this is good to go now, but let me know if you have any other thoughts

[kmdalton]

I think assign_resolution_bins should take an array of bin edges. i think refls that fall outside the bin range should probably get dropped by default, but i am open to suggestions.

[JBGreisman]

i think refls that fall outside the bin range should probably get dropped by default, but i am open to suggestions.

This is different than the current behavior in rs.utils.assign_with_binedges(), but could be implemented in DataSet.assign_resolution_bins()by pre-dropping any reflections before the call. If I do not do the pre-processing, the behavior will be to raise a ValueError that says "This function assumes 'bin_edges' contain every entry in 'data'" .

I hesitate to auto-drop reflections from the DataSet, but I think that could be acceptable as long we add a corresponding remark in the docstring

[JBGreisman]

I updated this PR to allow DataSet.assign_resolution_bins() to support making assignments based on explicit bin edges. I made the following decisions regarding API:

• Reflections outside of provided bin edges are dropped
• Bin edges can be provided in ascending or descending order, and assignments will depend on that order

[kmdalton]

looks good to me. great stuff!