Skip to content

Fix steam_lengthstem_length typo in DigRivers#84

Merged
spiani merged 2 commits into
no_ob_indices_anymorefrom
copilot/sub-pr-82
Mar 12, 2026
Merged

Fix steam_lengthstem_length typo in DigRivers#84
spiani merged 2 commits into
no_ob_indices_anymorefrom
copilot/sub-pr-82

Conversation

Copilot AI commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

_steam_length_to_stem and its steam_length parameter were a typo inconsistent with the stem_length config key and docstring terminology.

Changes

  • Rename _steam_length_to_stem_stem_length_to_stem
  • Rename parameter steam_lengthstem_length throughout the method
  • Fix error message and log string: "Steam length...""Stem length..."
  • Update call site to use the corrected method name

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Co-authored-by: spiani <8956990+spiani@users.noreply.github.com>
Copilot AI changed the title [WIP] [WIP] Address feedback on 'No ob indices anymore' pull request Fix steam_lengthstem_length typo in DigRivers Mar 12, 2026
@spiani spiani marked this pull request as ready for review March 12, 2026 14:28
Copilot AI review requested due to automatic review settings March 12, 2026 14:28
@spiani spiani merged commit e936f3d into no_ob_indices_anymore Mar 12, 2026
2 checks passed
@spiani spiani deleted the copilot/sub-pr-82 branch March 12, 2026 14:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes a typo in the DigRivers action by aligning helper naming, parameter naming, and user-facing strings with the existing stem_length terminology used in config and documentation.

Changes:

  • Renamed helper _steam_length_to_stem to _stem_length_to_stem.
  • Renamed helper parameter steam_length to stem_length throughout the method.
  • Updated log/error strings and the call site to use the corrected names.

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

You can also share your feedback on Copilot code review. Take the survey.

"Stem length must be a positive number of cell: received "
f"{stem_length}"
)
river_side = Direction(side)

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

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

The docstring says the river flows "in the direction opposite to its defined side", but the implementation uses river_side = Direction(side) and builds Movement(stem_length, river_side) (i.e., same direction as side). Please either update the docstring to match the actual movement direction, or change the code to use the opposite direction (e.g., -Direction(side)) if the doc is correct.

Suggested change
river_side = Direction(side)
river_side = -Direction(side)

Copilot uses AI. Check for mistakes.
Comment on lines 202 to 205
Returns:
A single `Movement`, representing the river's stem based on its
direction and length.

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

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

The Returns: section says this function returns "A single Movement", but the signature and implementation return a List[Movement] (a list containing one Movement). Update the docstring return description to match the actual return type/value.

Copilot uses AI. Check for mistakes.
raise ValueError(
"Steam length must be a positive number of cell: received "
f"{steam_length}"
"Stem length must be a positive number of cell: received "

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

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

The error message has a grammatical typo: "positive number of cell" should be "positive number of cells" (and possibly "cells:" vs "cells;" depending on style). This string is user-facing, so it’s worth correcting while touching it.

Suggested change
"Stem length must be a positive number of cell: received "
"Stem length must be a positive number of cells: received "

Copilot uses AI. Check for mistakes.
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.

3 participants