-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat(shade): Add property to track whether Shade is on the indoors #52
Conversation
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
@@ -548,14 +548,65 @@ def apertures_by_ratio_rectangle(self, ratio, aperture_height, sill_height, | |||
aperture._parent = self | |||
self._apertures.append(aperture) | |||
|
|||
def apertures_by_width_height_rectangle(self, aperture_height, aperture_width, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this needs to have the word repeating somewhere. It is easy to miss the s
in the call. I couldn't come up with a good short name and repeating_apertures_by_width_height_rectangle
is too long. Just an observation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it makes sense to use the word repeating
instead of rectangle
. Basically, I want to communicate that, just like apertures_by_ratio_rectangle
, this apertures_by_width_height_rectangle
method will fill any rectangular portions identified within the Face3D
with repeating rectangular windows. Let me think about this since a naming change like this should happen to both of these methods and will require PRs to a few others that are making use of them. If we eventually decide that we want to change it, I'll send another round of PRs.
This is_indoor property is needed to correctly set up the ModifierSet on honeybee-radiance. Also, as part of this commit, I am adding the ability to assign Shades to Doors. This has now come up enough times from more than one person that I've realized that I better implement it now while I still have a near-complete idea of all the other repositories affected by the change (another month or two and I don't think that I would be able to catch all of the places that will be incorrect as a result of this change). As far as I can tell, the following repos need to be changed with a PR that parallels this one: * honeybee-energy (especially the Model objectand Model writer) * honeybee-schema (including both Door schema and the Wiki) * energy-model-measure (OpenStudio has no concept of assigning Shades to Doors but I'll figure something out) * honeybee-grasshopper-core (most of the visualization components and the component that assigns Shade to a parent) Lastly, I added a new method to the Face to add repeating windows at a fixed width x height now that we have methods for this on ladybug_geometry. This doesn't require any parallel PRs in other repos but, at some point, I should probably add a WindowParameter object to Dragonfly that makes use of this new method.
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
🎉 This PR is included in version 1.21.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
This commit brings the library in line with [this PR on honeybee-core](ladybug-tools/honeybee-core#52).
This is_indoor property is needed to correctly set up the ModifierSet on honeybee-radiance.
Also, as part of this commit, I am adding the ability to assign Shades to Doors. This has now come up enough times from more than one person that I've realized that I better implement it now while I still have a near-complete idea of all the other repositories affected by the change (another month or two and I don't think that I would be able to catch all of the places that will be incorrect as a result of this change). As far as I can tell, the following repos need to be changed with a PR that parallels this one:
Lastly, I added a new method to the Face to add repeating windows at a fixed width x height now that we have methods for this on ladybug_geometry. This doesn't require any parallel PRs in other repos but, at some point, I should probably add a WindowParameter object to Dragonfly that makes use of this new method.