Skip to content

fix: use canonical structure() attribute names#2755

Merged
krlmlr merged 2 commits into
mainfrom
claude/fix-structure-attr-names
Jul 26, 2026
Merged

fix: use canonical structure() attribute names#2755
krlmlr merged 2 commits 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.

Empty commit to re-run the checks, in particular the touchstone benchmarks.

Co-Authored-By: Claude Opus 5 <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 ac4a9e5 is merged into main:

  • ✔️as_adjacency_matrix: 743ms -> 743ms [-0.33%, +0.24%]
  • ✔️as_biadjacency_matrix: 724ms -> 723ms [-0.56%, +0.31%]
  • ✔️as_data_frame_both: 1.43ms -> 1.43ms [-1.1%, +0.83%]
  • ✔️as_long_data_frame: 3.82ms -> 3.82ms [-0.65%, +0.63%]
  • ✔️es_attr_filter: 2.61ms -> 2.61ms [-0.36%, +0.65%]
  • ✔️graph_from_adjacency_matrix: 116ms -> 116ms [-0.84%, +1.93%]
  • ✔️graph_from_data_frame: 3.26ms -> 3.25ms [-0.89%, +0.51%]
  • ✔️vs_attr_filter: 1.46ms -> 1.47ms [-0.14%, +1.86%]
  • ✔️vs_by_name: 936µs -> 941µs [-0.37%, +1.38%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@krlmlr

krlmlr commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Reran checks to measure performance. Even if it was a problem here, we can't do very much about it.

@krlmlr
krlmlr enabled auto-merge (squash) July 26, 2026 09:15
@krlmlr
krlmlr merged commit fb24656 into main Jul 26, 2026
9 checks passed
@krlmlr
krlmlr deleted the claude/fix-structure-attr-names branch July 26, 2026 09:15
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