ENH: Prevent out of bounds Tanks from Instantiation #484
Conversation
phmbressan
added
Bug
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## hotfix/v1.1.1 #484 +/- ##
=================================================
+ Coverage 70.83% 70.84% +0.01%
=================================================
Files 55 55
Lines 9240 9258 +18
=================================================
+ Hits 6545 6559 +14
- Misses 2695 2699 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
rocketpy/motors/tank.py
Outdated
| """ | ||
| if (self.fluid_volume > self.geometry.total_volume + 1e-6).any(): | ||
| raise ValueError( | ||
| f"The tank '{self.name}' is overfilled. The fluid volume is " |
There was a problem hiding this comment.
maybe you could create a single string to be used in both error messages, thus reducing the number of lines here.
Something like this.
error_msg = "The tank '{self.name}' is {fill_condition}'. " + "Consider increasing input fluid quantities or checking the fluid density values.\n\t\t" + "The fluid volume is {volume:.3f} m³ at {time:.3f} s."
raise ValueError(error_msg.format(volume=self.fluid_volume.y_array[max_volume_idx],
time=self.fluid_volume.x_array[max_volume_idx]))
Sorry for keeping this short.
rocketpy/motors/tank.py
Outdated
| raise ValueError( | ||
| f"The tank '{self.name}' is overfilled. The fluid volume is " | ||
| + "greater than the total volume of the tank.\n\t\t" | ||
| + "Try increasing the tank height and check out the fluid density" | ||
| + " values.\n\t\t" | ||
| + f"The fluid volume is {np.max(self.fluid_volume.y_array):.3f} m³ at " | ||
| + f"{self.fluid_volume.x_array[np.argmax(self.fluid_volume.y_array)]:.3f} s." | ||
| ) |
There was a problem hiding this comment.
| raise ValueError( | |
| f"The tank '{self.name}' is overfilled. The fluid volume is " | |
| + "greater than the total volume of the tank.\n\t\t" | |
| + "Try increasing the tank height and check out the fluid density" | |
| + " values.\n\t\t" | |
| + f"The fluid volume is {np.max(self.fluid_volume.y_array):.3f} m³ at " | |
| + f"{self.fluid_volume.x_array[np.argmax(self.fluid_volume.y_array)]:.3f} s." | |
| ) | |
| raise ValueError( | |
| f"The tank '{self.name}' is overfilled. The fluid volume is " | |
| + "greater than the total volume of the tank.\n\t\t" | |
| + "Try increasing the tank height and check out the fluid density" | |
| + " values.\n\t\t" | |
| + f"The fluid volume is {np.max(self.fluid_volume.y_array):.3f} m³ at " | |
| + f"{self.fluid_volume.x_array[np.argmax(self.fluid_volume.y_array)]:.3f} s.\n\t\t" | |
| + f"The tank total volume is {self.geometry.total_volume:.3f} m³." | |
| ) |
There was a problem hiding this comment.
I suggest that we also print the tank total volume in all the over/underfilled error messages
|
Hey, last comment, should we add tests to ensure the exceptions are being raised correctly? |
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest --runslow) have passed locallyCurrent behavior
Due to the fact that the
Tankgetters are evaluated lazily (post instance), it is possible that aTankwith invalid inputs is only detected by specific methods that may not be called.New behavior
This PR introduces methods to check if the
Tankfluids are inside the appropriate volume limits and calls them on instantiation.Therefore, a invalid
Tankshall not be constructed.Breaking change