JWock82 / PyNite

A 3D structural engineering finite element library for Python.
MIT License
421 stars 86 forks source link

Results arrays (#144) #145

Closed connorferster closed 1 year ago

connorferster commented 1 year ago

Addresses #144 with the following changes:

connorferster commented 1 year ago

@JWock82

Let me know your thoughts on this and if you would like any other additions to the code base to support this proposed change.

JWock82 commented 1 year ago

I like this. It seems like shear_array and plot_shear are similar to each other and this could be done in plot_shear with an argument that switches the output from matplotlib to an array. If you want to merge the methods together feel free and then issue another PR. Otherwise I'll merge this PR and then merge the methods myself when I find a spare moment in the next week or so.

connorferster commented 1 year ago

Glad you like it!

Agreed, the "array" functions and the "plot" methods are similar and the implementation code is repetitive. I hear what you are saying on adding a boolean flag to the plot methods. My only misgiving with such an approach is that it would make the return type of the plot functions inconsistent which I think is unexpected. If (I mean "when") I was cruising the API to see what method to use to get a result array, the plot methods would not be my first guess.

However, I think that reducing the amount of repetition would be good because it reduces the quantity of code requiring maintenance. Let me know what you think of this counter proposal:

Add a method like this:

def _results_array(self, action: str, n_points: int, direction: Optional[str] = None, combo_name="Combo 1") -> np.ndarray:
    """
    Returns the results array according to the parameters.

    action: {"shear", "moment", "torque", "axial", "deflection", "rel_deflection"}
    n_points: the number of points
    direction: the direction of action to retrieve
    combo_name: the load combination to retrieve
    """
    action_function = getattr(self, action)
    L = self.L()
    x_arr = linspace(0, L, n_points)
    if direction is None:
            y_arr = array(
                [self.action_function(x, combo_name) for x in x_arr]
            )
    else:
            y_arr = array(
                [self.action_function(direction, x, combo_name) for x in x_arr]
            )
    return array([x_arr, y_arr])

Then, the implementation of each array function simply becomes:

def shear_array(direction: str, n_points: int, combo_name="Combo1") -> array:
    """
    ...doc string
    """
    return self._results_array("shear", direction, n_points, combo_name)

The plot functions could then call the _results_array methods in a similar fashion. This would leave, effectively, only one method that requires maintenance. If you wanted to take it further, you could do the same with the plot methods: abstract it into a single _results_plot method (for example) and then each plot method would simply call that, each with the appropriate inputs. Then you would only have two methods that need maintaining and you have created convenient and intuitive API endpoints for the user (i.e. plot_shear and shear_array, etc.)

Since these methods would be "private" methods, and only called internally by other methods, you could also do away with a lot of error checking (like checking if the action string passed was a valid action name). The current error checking in all of the plot functions could be moved to the _results_array method, further reducing the maintenance surface.

Let me know your thoughts on this and I can put it together.

JWock82 commented 1 year ago

I see what you're getting at, and I'll probably move it that direction. I've merged your PR for now.