Skip to content

docs(surfinfo): add missing docstring for flat_border - #694

Merged
anujanegi merged 2 commits into
gallantlab:mainfrom
evi-hendrikx:docs/surfinfo-docstrings
Sep 21, 2026
Merged

anujanegi merged 2 commits into
gallantlab:mainfrom
evi-hendrikx:docs/surfinfo-docstrings

Conversation

@AgarwalNilay

Copy link
Copy Markdown
Contributor

flat_border had no docstring at all despite being on the website's API reference. Describes its outfile/subject parameters and the lines/ismwalls arrays it saves.

AgarwalNilay and others added 2 commits August 24, 2026 09:37
flat_border had no docstring at all despite being on the website's
API reference. Describes its outfile/subject parameters and the
lines/ismwalls arrays it saves.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
flat_border references an undefined `height` variable, which raises
NameError as written. Adds a FIXME comment noting this and pointing
to the commented-out Image.new(...) call that suggests it was meant
to be a parameter, for a reviewer to decide the right fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@AgarwalNilay
AgarwalNilay force-pushed the docs/surfinfo-docstrings branch from 010f43a to 6c02855 Compare August 24, 2026 16:37
@sjshim sjshim self-assigned this Aug 31, 2026

@anujanegi anujanegi left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Verified the new docstring against the implementation line-by-line:

  • lines entries are pts[pbnd, :2] — 2D point arrays, matches the description.
  • ismwalls entries are the pmw booleans — matches.
  • Style/format is consistent with the other docstrings in this file (curvature, distortion, thickness, tissots_indicatrix): same Parameters layout, no Returns section (correct, since these functions only write to outfile).

Also checked the FIXME comment about the undefined height name: flat_border has no callers or tests anywhere in the codebase, and there's no existing tracking issue for it, so this is dead/broken legacy code. Documenting the landmine rather than guessing at a fix is the right call for a docs-scoped PR — no behavior change, low risk.

CI is green across the board. Approving.


Reviewed with Claude Code assistance.

@anujanegi
anujanegi merged commit 845aa13 into gallantlab:main Sep 21, 2026
14 checks passed
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.

4 participants