Skip to content

fix: use canonical structure() attribute names#2755

Open
krlmlr wants to merge 1 commit into
mainfrom
claude/fix-structure-attr-names
Open

fix: use canonical structure() attribute names#2755
krlmlr wants to merge 1 commit into
mainfrom
claude/fix-structure-attr-names

Conversation

@krlmlr

@krlmlr krlmlr commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

LLM disclosure: this PR was prepared with Claude Code (Claude Opus 4.8).

  • By submitting this pull request, I assign the copyright of my contribution to The igraph development team.

What

R-devel deprecates the special structure() arguments .Names, .Dim and .Dimnames; they emit warnings that the test suite escalates (the ~27 warnings noted on #2749).

Replace them with the canonical names, dim and dimnames. structure() maps both spellings to the same attributes, so the constructed objects are identical — no behavior change.

  • 42 occurrences: 9 tests/testthat/*.R fixtures (mostly dput() output) and one R/plot.common.R fixture.
  • Each replacement verified to be a structure() attribute argument — the list(...) data and its inner element names are untouched.
  • All edited files parse cleanly.

Scope

Code changes only, branched off main. The CI/workflow side of the R-devel work lives separately:

🤖 Generated with Claude Code


Generated by Claude Code

R-devel deprecates the special structure() arguments .Names, .Dim and
.Dimnames, which emit warnings that the test suite escalates.

Replace them with the canonical names, dim and dimnames in the affected test
fixtures and one plot fixture. structure() maps both spellings to the same
attributes, so the constructed objects are identical -- there is no behavior
change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019E6CUgb5aFw6uvWXztcHgV
@github-actions

Copy link
Copy Markdown
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 8ced78f is merged into main:

  • ✔️as_adjacency_matrix: 788ms -> 785ms [-2.13%, +1.19%]
  • ✔️as_biadjacency_matrix: 755ms -> 752ms [-2.22%, +1.63%]
  • ✔️as_data_frame_both: 1.43ms -> 1.47ms [-2.23%, +7.94%]
  • ✔️as_long_data_frame: 3.52ms -> 3.52ms [-4%, +4%]
  • ✔️es_attr_filter: 2.68ms -> 2.71ms [-3.36%, +6.12%]
  • ✔️graph_from_adjacency_matrix: 148ms -> 147ms [-2.42%, +0.87%]
  • ❗🐌graph_from_data_frame: 3.06ms -> 3.13ms [+0.03%, +4.78%]
  • ✔️vs_attr_filter: 1.49ms -> 1.5ms [-3.81%, +4.24%]
  • ✔️vs_by_name: 997µs -> 1ms [-4.01%, +4.72%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

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.

2 participants