Skip to content

Conversation

@samay2504
Copy link

fixes #248.

Solution

Deprecated shape_index() in favor of radii_ratio() with:

  • Clear migration path: Deprecation warning with actionable message
  • Backward compatibility: No breaking changes; existing code continues to work with a warning
  • Consistent naming: radii_ratio better expresses the metric's mathematical structure
  • Comprehensive docs: Updated radii_ratio() docstring to note it's also known as Schumm's shape index

Changes

  1. esda/shape.py:

    • Added import warnings at module level
    • shape_index() now emits DeprecationWarning and delegates to radii_ratio()
    • Enhanced radii_ratio() docstring with mathematical formula and historical notes
    • Added "See Also" section cross-referencing deprecated function
  2. esda/tests/test_shape.py:

    • Updated test_shape_index() to verify deprecation warning is properly raised
    • Test confirms both functions produce identical results
    • Added import for warnings module

Testing

Both test_shape_index() and test_radii_ratio() pass
Deprecation warning correctly raised with stacklevel=2
No regressions in any shape module tests
Code formatted with ruff and black

- shape_index() is identical to radii_ratio()
- Added deprecation warning to shape_index() with clear migration path
- Updated radii_ratio() docstring to clarify it is also known as Schumm's shape index
- Added comprehensive test to verify deprecation warning is raised
- Test confirms shape_index() and radii_ratio() produce identical results
- Formatted with ruff and black

Closes pysal#248
Copilot AI review requested due to automatic review settings January 17, 2026 19:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR deprecates the shape_index() function in favor of radii_ratio() to establish clearer naming that better expresses the metric's mathematical structure, while maintaining full backward compatibility.

Changes:

  • Added deprecation warning to shape_index() that delegates to radii_ratio()
  • Enhanced radii_ratio() docstring with mathematical formula, historical context, and cross-reference
  • Updated test to verify deprecation warning is properly raised and both functions produce identical results

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
esda/shape.py Added warnings import; deprecated shape_index() with clear migration path; enhanced radii_ratio() docstring with formula and "See Also" section
esda/tests/test_shape.py Added warnings import; updated test_shape_index() to verify deprecation warning and equivalence with radii_ratio()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


Also known as Schumm's shape index (Schumm (1956) in MacEachren 1985).

The ratio of the radius of the equi-areal circle to the radius of the MBC
Copy link

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing period at the end of the description sentence. The line should end with a period for consistency with other docstrings in the module.

Suggested change
The ratio of the radius of the equi-areal circle to the radius of the MBC
The ratio of the radius of the equi-areal circle to the radius of the MBC.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Jan 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.7%. Comparing base (bb0f069) to head (9200a49).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #408   +/-   ##
=====================================
  Coverage   82.7%   82.7%           
=====================================
  Files         27      27           
  Lines       3829    3830    +1     
=====================================
+ Hits        3166    3167    +1     
  Misses       663     663           
Files with missing lines Coverage Δ
esda/shape.py 77.2% <100.0%> (+0.1%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines 182 to 190
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
observed = esda.shape.shape_index(shape)

# Check that deprecation warning was raised
assert len(w) == 1
assert issubclass(w[0].category, DeprecationWarning)
assert "deprecated" in str(w[0].message).lower()
assert "radii_ratio" in str(w[0].message)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use pytest.warns and check the message match. This is unnecessary.

Comment on lines +192 to +194
# Check that result matches radii_ratio
expected = esda.shape.radii_ratio(shape)
testing.assert_allclose(observed, expected, atol=ATOL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the original test here. You are deprecating stuff, you should ensure that original tests pass.

testing.assert_allclose(observed, 0.659366, atol=ATOL)

esda/shape.py Outdated
"""
Schumm’s shape index (Schumm (1956) in MacEachren 1985)
.. deprecated:: 2.5.0
``shape_index`` is deprecated and will be removed in version 3.0.0.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
``shape_index`` is deprecated and will be removed in version 3.0.0.
``shape_index`` is deprecated and will be removed in future.

Might be before 3.0.0.

esda/shape.py Outdated
Schumm’s shape index (Schumm (1956) in MacEachren 1985)
.. deprecated:: 2.5.0
``shape_index`` is deprecated and will be removed in version 3.0.0.
Use ``radii_ratio`` instead, which computes the same Schumm's shape index
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Use ``radii_ratio`` instead, which computes the same Schumm's shape index
Use :func:`radii_ratio` instead, which computes the same Schumm's shape index

This shall give a link.

esda/shape.py Outdated
ga = _cast(collection)
return numpy.sqrt(shapely.area(ga) / numpy.pi) / shapely.minimum_bounding_radius(ga)
warnings.warn(
"shape_index is deprecated and will be removed in version 3.0.0. "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"shape_index is deprecated and will be removed in version 3.0.0. "
"shape_index is deprecated and will be removed in future. "

@martinfleis martinfleis changed the title fix: Deprecate shape_index() in favor of radii_ratio() to resolve DEPR: Deprecate shape_index() in favor of radii_ratio() Jan 18, 2026
… deprecation messages to 'future', add func cross-references
@samay2504
Copy link
Author

@martinfleis I have applied the changes you have asked for so please review them and tell me if you want anything else modified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

shape_index is the same as radii_ratio

2 participants