Skip to content

Conversation

@wpbonelli
Copy link
Member

@wpbonelli wpbonelli commented Jan 8, 2026

Previously the Grid.get_cell_vertices() method looked/worked different depending on the grid type. Make it consistent across all three grid types by supporting both cellid and node keyword arguments and being lenient about what's passed to cellid i.e. accept a node number, a fully-sized tuple, or an "abbreviated" tuple lacking layer index. Cell ID and node number are conceptually distinct but til now VertexGrid and UnstructuredGrid methods had a single parameter cellid supporting (for vertex) or expecting (for unstructured) a node number, which we don't want to break. And continue also supporting i and j for StructuredGrid.

Noticed these inconsistencies when working on #2677

@wpbonelli wpbonelli added the refactor Non-functional changes label Jan 8, 2026
@wpbonelli wpbonelli added this to the 3.10 milestone Jan 8, 2026
@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

❌ Patch coverage is 71.42857% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.7%. Comparing base (556c088) to head (3020a90).
⚠️ Report is 103 commits behind head on develop.

Files with missing lines Patch % Lines
flopy/discretization/structuredgrid.py 68.4% 6 Missing ⚠️
flopy/discretization/vertexgrid.py 73.6% 5 Missing ⚠️
flopy/discretization/unstructuredgrid.py 72.7% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2678      +/-   ##
===========================================
+ Coverage     55.5%    72.7%   +17.1%     
===========================================
  Files          644      667      +23     
  Lines       124135   129706    +5571     
===========================================
+ Hits         68947    94341   +25394     
+ Misses       55188    35365   -19823     
Files with missing lines Coverage Δ
flopy/discretization/unstructuredgrid.py 76.8% <72.7%> (-4.7%) ⬇️
flopy/discretization/vertexgrid.py 80.1% <73.6%> (-3.5%) ⬇️
flopy/discretization/structuredgrid.py 53.1% <68.4%> (+5.6%) ⬆️

... and 557 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wpbonelli wpbonelli marked this pull request as ready for review January 8, 2026 17:44
----------
cellid : int or tuple, optional
Cell identifier. Can be:
- node number (int)
Copy link
Member Author

Choose a reason for hiding this comment

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

Cell ID and node number are conceptually distinct but up to now the VertexGrid and UnstructuredGrid versions of this method had a single parameter cellid supporting (for vertex) or expecting (for unstructured) a node number. So I think for consistency we can allow passing node number to cellid for StructuredGrid too.

@wpbonelli wpbonelli merged commit aaa38e7 into modflowpy:develop Jan 8, 2026
20 checks passed
@wpbonelli wpbonelli deleted the get-cell-vertices branch January 8, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Non-functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant