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

added generic fastener element #310

Merged
merged 21 commits into from
Nov 14, 2024
Merged

added generic fastener element #310

merged 21 commits into from
Nov 14, 2024

Conversation

obucklin
Copy link
Contributor

@obucklin obucklin commented Oct 28, 2024

closes #319

This adds a generic fastener element which sets the stage for carious connectors like brackets and steel plates. We may want to add another generic 'dowel' class or we could include screws, nails, dowels, etc here under fastener. Heirarchy TBD.

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke lint).
  • I added new functions/classes and made them available on a second-level import, e.g. compas_timber.datastructures.Beam.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

@obucklin
Copy link
Contributor Author

obucklin commented Oct 28, 2024

i'm not sure what

"I added new functions/classes and made them available on a second-level import, e.g. compas_timber.datastructures.Beam"

means, so I clicked it

@obucklin
Copy link
Contributor Author

Now with 100,000,000,000,000,000,000% more unit tests!

Copy link
Contributor

@papachap papachap left a comment

Choose a reason for hiding this comment

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

Now with 100,000,000,000,000,000,000% more unit tests!

haha I see that Chen's presentation yesterday really had an impact on you!

src/compas_timber/_fabrication/step_joint.py Outdated Show resolved Hide resolved
src/compas_timber/_fabrication/step_joint_notch.py Outdated Show resolved Hide resolved
src/compas_timber/elements/fasteners/fastener.py Outdated Show resolved Hide resolved
papachap

This comment was marked as duplicate.

@chenkasirer chenkasirer mentioned this pull request Nov 11, 2024
5 tasks
Copy link
Contributor

@chenkasirer chenkasirer left a comment

Choose a reason for hiding this comment

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

I am still confused about Fastener it looks like a partial copy of Element and Beam but not clear how it's used. From our whiteboard session I'm missing the FastenerJoint part and how it interacts with FastenerElement. Should we have another session to discuss?

src/compas_timber/_fabrication/step_joint.py Outdated Show resolved Hide resolved
src/compas_timber/elements/fasteners/fastener.py Outdated Show resolved Hide resolved
src/compas_timber/elements/fasteners/fastener.py Outdated Show resolved Hide resolved
src/compas_timber/elements/fasteners/fastener.py Outdated Show resolved Hide resolved
src/compas_timber/elements/fasteners/fastener.py Outdated Show resolved Hide resolved
src/compas_timber/elements/fasteners/fastener.py Outdated Show resolved Hide resolved
src/compas_timber/elements/fasteners/fastener.py Outdated Show resolved Hide resolved
src/compas_timber/elements/fasteners/fastener.py Outdated Show resolved Hide resolved
@obucklin
Copy link
Contributor Author

ready for re-review

@obucklin
Copy link
Contributor Author

I am still confused about Fastener it looks like a partial copy of Element and Beam but not clear how it's used. From our whiteboard session I'm missing the FastenerJoint part and how it interacts with FastenerElement. Should we have another session to discuss?

I don't see the need for a separate FastenerJoint ATM. do you?

@chenkasirer
Copy link
Contributor

I am still confused about Fastener it looks like a partial copy of Element and Beam but not clear how it's used. From our whiteboard session I'm missing the FastenerJoint part and how it interacts with FastenerElement. Should we have another session to discuss?

I don't see the need for a separate FastenerJoint ATM. do you?

ah I think what's happening is that you're doing all that in that other PR (BallFastenerJoiningBallWithTentaclesJoint). so maybe that's why I'm confused.

@obucklin
Copy link
Contributor Author

I am still confused about Fastener it looks like a partial copy of Element and Beam but not clear how it's used. From our whiteboard session I'm missing the FastenerJoint part and how it interacts with FastenerElement. Should we have another session to discuss?

I don't see the need for a separate FastenerJoint ATM. do you?

ah I think what's happening is that you're doing all that in that other PR (BallFastenerJoiningBallWithTentaclesJoint). so maybe that's why I'm confused.

tryna keep it grainular

Copy link
Contributor

@chenkasirer chenkasirer left a comment

Choose a reason for hiding this comment

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

LGTM

CHANGELOG.md Outdated Show resolved Hide resolved
@obucklin obucklin merged commit 7c21e44 into main Nov 14, 2024
14 checks passed
@obucklin obucklin deleted the fastener_element branch November 14, 2024 14:25
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.

Fastener generic element type
3 participants