pvlib / twoaxistracking

twoaxistracking is a python package for simulating two-axis tracking solar collectors, particularly self-shading.
https://twoaxistracking.readthedocs.io
BSD 3-Clause "New" or "Revised" License
8 stars 4 forks source link

Add TwoAxisTrackerField class #14

Closed AdamRJensen closed 2 years ago

AdamRJensen commented 2 years ago

This PR adds a class called TwoAxisTrackerField which is a convient container for the collector geometry and field layout.

The class can be used for most use cases and significantly simplifies the simulation process. The shaded fraction for a range of solar elevation and azimuth angles can be calculated using the class by:

two_axis_tracker_field.get_shaded_fraction(solar_elevation, solar_azimuth)
AdamRJensen commented 2 years ago

The previous commits corrects the issue in the equation of the horizon shading as described in #18.

wholmgren commented 2 years ago

Have you considered implementing this as a dataclass? Looks like a good fit to me.

AdamRJensen commented 2 years ago

Have you considered implementing this as a dataclass? Looks like a good fit to me.

@wholmgren I have not, for the simple reason that it is not straightforward to me what the benefit is...

kandersolar commented 2 years ago

Have you considered implementing this as a dataclass? Looks like a good fit to me.

Just curious, what's the benefit of making it a dataclass? As currently written it needs a custom constructor, so that boilerplate can't be gotten rid of. None of the other dataclass functionality seems particularly appealing to me in this case.

wholmgren commented 2 years ago

Comment was mostly based on the docstring "TwoAxisTrackerField is a convenient container for the collector geometry and field layout" and a too-quick glance at the code. In general I think dataclasses are easier to read, allow type hints with no cost to readability, and encourage simple class design. The custom stuff could be achieved with a __post_init__ function, but the readability gains at that point are not obvious. So, was just an idea and it's fine to ignore.