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

Allow unlipped cee and zed steel_sections #300

Merged

Conversation

smith120bh
Copy link
Contributor

This PR recreates #285 , but is based upon the hypermodern branch from the outset instead!

--- Original description below ---
Unlipped Cee sections are very common. For example, all track ("T") sections in the Steel Framing Industry Association standard sections, as defined here, are unlipped Cee sections. However, section-properties currently explicitly throws an error if lip length l is set to anything less than r_out (the outer radius).

This pull request adds the ability to set l to zero - or anything less than r_out.

Note that I did have a question as to what to do for 0 < l < r_out. One option would be to throw an error in this case, but I opted instead for decreasing the outer radius commensurately, on the philosophy that fewer error conditions is probably better, but am happy to change this as well.

While unlipped Zed sections are less common than unlipped Cees, they do exist - and a nearly-identical update could be applied to zed_section() to allow unlipped zeds as well here.

@robbievanleeuwen robbievanleeuwen added the enhancement New feature or request label Oct 3, 2023
@smith120bh
Copy link
Contributor Author

@robbievanleeuwen
Blackify

Sorry about that! I missed that my black extension didn't format on save here.

@robbievanleeuwen
Copy link
Owner

robbievanleeuwen commented Oct 3, 2023

All good @smith120bh, contributor guide is pretty vague at the moment - once i get this big PR merged using poetry and pre-commit will make contributing much easier and standardised.

I've also just removed the ValueError thrown if l < r_out which I assume is no longer relevant? EDIT - all good, I see this was in the previous PR 👍

I've had a play around with a number of edge cases and it seems to work well, thanks!

@robbievanleeuwen robbievanleeuwen merged commit 16909ba into robbievanleeuwen:hypermodern Oct 3, 2023
13 checks passed
@smith120bh smith120bh deleted the hypermodern branch October 3, 2023 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants