Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add major_radius bool to HexLocations and rename apothem parameter to radius #643

Merged
merged 6 commits into from
Jun 20, 2024

Conversation

jdegenstein
Copy link
Collaborator

This PR fixes #633 by adding a major_radius: bool to HexLocations that defaults to False (to preserve current behavior). This PR is technically breaking because the prior apothem parameter had to be named to a more generic radius which can be either a major radius or minor radius (aka apothem).

Here is an example of the new functionality:

hex = RegularPolygon(1, 6)
with BuildSketch() as s:
    with HexLocations(1, 3, 3, False) as hloc:
        add(hex)
    with Locations((6,0)):
        with HexLocations(1, 3, 3, True) as hloc2:
            add(hex)

image

@gumyr
Copy link
Owner

gumyr commented Jun 19, 2024

Looks good to me. Your use case for this was something I didn't envision but is perfectly valid. Two of the examples would be broken though.

@jdegenstein jdegenstein marked this pull request as draft June 20, 2024 01:30
@jdegenstein jdegenstein marked this pull request as ready for review June 20, 2024 01:42
@jdegenstein
Copy link
Collaborator Author

Fixed the two examples and merged.

@jdegenstein jdegenstein merged commit 29ad538 into gumyr:dev Jun 20, 2024
11 checks passed
@jdegenstein jdegenstein deleted the hexloc_majradius branch June 20, 2024 01:52
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 major_radius bool to HexLocations
2 participants