Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 26, 2025

Implementation Complete ✅

Successfully implemented make_hex_lattice() and hex_lattice() functions using igraph_hexagonal_lattice().

Changes Made

  • Merge main branch (latest updates)
  • Follow updated AGENTS.md instructions
  • Clarified documentation: Fixed confusing documentation that incorrectly suggested hexagonal and triangular lattices are the same thing
  • Updated snapshots to match new error formatting
  • Refactored attribute handling: Changed from direct assignment to using set_graph_attr()
  • Updated function signatures: Added ellipsis before optional parameters and used explicit arguments in constructor specs
  • Added check_dots_empty() to both functions for proper validation
  • Specified GATTR in functions-R.yaml for future use when function is added to C interface
  • Used dimvector for attribute name to match C function parameter and GATTR specification
  • All tests pass (116 tests in test-make.R, 1167 tests in test-aaa-auto.R)

Key Documentation Fix

Before: The documentation incorrectly stated that hexagonal lattice is "also sometimes referred to as a triangular lattice"

After: Corrected documentation clarifies that:

  • Hexagonal lattice and triangular lattice are different structures
  • They are planar duals of each other
  • make_hex_lattice() implements the hexagonal lattice (degree 3 vertices)
  • The triangular lattice is available via igraph_triangular_lattice() in the C library

Function Signature Updates

Updated signatures following tidyverse design guide:

# Main function with ellipsis before optional args
make_hex_lattice <- function(dims, ..., directed = FALSE, mutual = FALSE) {
  check_dots_empty()
  # ...
}

# Constructor spec with explicit arguments and dots checking
hex_lattice <- function(dims, ..., directed = FALSE, mutual = FALSE) {
  check_dots_empty()
  constructor_spec(make_hex_lattice, dims = dims, directed = directed, mutual = mutual)
}

This follows the pattern where:

  • Required arguments come first
  • ... comes before optional arguments (for future extensibility)
  • check_dots_empty() validates no unexpected arguments are passed
  • Constructor specs use explicit arguments instead of passing ...

GATTR Specification

Added GATTR specification in tools/stimulus/functions-R.yaml:

igraph_hexagonal_lattice:
    R:
      GATTR:
        name: Hexagonal lattice
      GATTR-PARAM: dimvector, directed, mutual

This documents the intended attribute handling. When igraph_hexagonal_lattice is added to the base C interface definitions, the function can be auto-generated with proper GATTR support.

Implementation Summary

This PR exposes the igraph_hexagonal_lattice() C function through two new R functions:

  1. make_hex_lattice(): Creates hexagonal lattices

    • Uses hexagonal_lattice_impl() which calls C igraph_hexagonal_lattice()
    • Supports three boundary shapes: triangular, rectangular, hexagonal
    • Supports directed and mutual edge options
    • Marked as experimental
    • Attributes follow GATTR specification
  2. hex_lattice(): Constructor spec for make_() infrastructure

  3. Comprehensive tests with correct expectations

Function Usage

# Triangular boundary: 22 vertices, 27 edges
make_hex_lattice(3)

# Rectangular boundary: 38 vertices, 49 edges
make_hex_lattice(c(3, 4))

# Hexagonal boundary: 24 vertices, 30 edges
make_hex_lattice(c(2, 2, 2))

# With make_() infrastructure
make_(hex_lattice(3))

All functionality works correctly and follows updated code style guidelines!

Original prompt

This section details on the original issue you should resolve

<issue_title>Add make_hex_lattice()</issue_title>
<issue_description>Expose igraph_triangular_lattice().

Suggested name: make_hex_lattice() along with hex_lattice(), which should work with make_().

</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Add make_hex_lattice function to expose igraph_triangular_lattice feat: add make_hex_lattice() and hex_lattice() functions Oct 26, 2025
Copilot AI requested a review from krlmlr October 26, 2025 14:03
Copilot finished work on behalf of krlmlr October 26, 2025 14:03
Copy link
Contributor

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

@copilot: Use igraph_hexagonal_lattice, not triangular. Merge main and obey updated instructions.

@krlmlr
Copy link
Contributor

krlmlr commented Oct 26, 2025

@copilot: Merge main, resolve conflicts as needed, and obey updated instructions. Use igraph_hexagonal_lattice, not triangular.

@krlmlr
Copy link
Contributor

krlmlr commented Nov 1, 2025

@copilot: Merge main, resolve conflicts, and follow updated AGENTS.md .

Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot: Merge main, resolve conflicts, and follow updated AGENTS.md .

Completed in 617ba77. Merged main, resolved conflicts, and updated implementation to use igraph_hexagonal_lattice() following updated AGENTS.md guidelines. All tests pass.

Copilot AI requested a review from krlmlr November 1, 2025 11:29
Copilot finished work on behalf of krlmlr November 1, 2025 11:29
@krlmlr
Copy link
Contributor

krlmlr commented Nov 1, 2025

@copilot: Merge main and follow updated AGENTS.md . Double-check triangular and hexagonal in the documentation.

Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot: Merge main and follow updated AGENTS.md . Double-check triangular and hexagonal in the documentation.

Completed in acab512. Merged main and fixed the documentation - hexagonal and triangular lattices are different structures (planar duals), not the same thing as the previous docs incorrectly suggested. All tests pass.

Copilot finished work on behalf of krlmlr November 1, 2025 16:07
@krlmlr
Copy link
Contributor

krlmlr commented Nov 1, 2025

@copilot: Change in the spirit of #2466.

Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot: Change in the spirit of #2466.

Completed in 77ec3fc. Changed from direct attribute assignment to using set_graph_attr(), following the pattern from #2466 and the updated AGENTS.md guidelines. All tests pass.

Copilot finished work on behalf of krlmlr November 1, 2025 19:15
@krlmlr
Copy link
Contributor

krlmlr commented Nov 1, 2025

@copilot: Prefer GATTR, use explicit arguments instead of passing on ellipsis. Add ellipsis before optional arguments.

Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot: Prefer GATTR, use explicit arguments instead of passing on ellipsis. Add ellipsis before optional arguments.

Completed in 7f33188. Updated function signatures to use explicit arguments with ellipsis before optional parameters. Changed hex_lattice() from passing ... to explicitly listing arguments. Cannot use GATTR yet as igraph_hexagonal_lattice is not in the base C interface definitions. All tests pass.

Copilot finished work on behalf of krlmlr November 1, 2025 19:58
#' @rdname make_hex_lattice
#' @export
hex_lattice <- function(dims, ..., directed = FALSE, mutual = FALSE) {
constructor_spec(make_hex_lattice, dims = dims, directed = directed, mutual = mutual)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
constructor_spec(make_hex_lattice, dims = dims, directed = directed, mutual = mutual)
check_dots_empty()
constructor_spec(make_hex_lattice, dims = dims, directed = directed, mutual = mutual)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Completed in a5e6b2f. Added check_dots_empty() to hex_lattice() as suggested.

Copy link
Contributor

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

@copilot: See review, use @inheritParams, specify GATTR in functions-R.yaml .

Copy link
Contributor Author

Copilot AI commented Nov 1, 2025

@copilot: See review, use @inheritParams, specify GATTR in functions-R.yaml .

Completed in a5e6b2f. Added check_dots_empty() to hex_lattice(), specified GATTR in functions-R.yaml, and changed attribute name from dims to dimvector to match GATTR-PARAM specification. All tests pass.

Copilot AI requested a review from krlmlr November 1, 2025 20:19
Copilot finished work on behalf of krlmlr November 1, 2025 20:19
@krlmlr
Copy link
Contributor

krlmlr commented Nov 1, 2025

@szhorvat: Copilot complains that

Cannot use GATTR yet as igraph_hexagonal_lattice is not in the base C interface definitions. ...

What's the process here?

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.

Add make_hex_lattice()

2 participants