Set all ImpactFunc attributes in __init__ #550
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peanutfun
commented
Oct 25, 2022
Comment on lines
+58
to
+67
| def __init__( | ||
| self, | ||
| haz_type: str = "", | ||
| id: Union[str, int] = "", | ||
| intensity: Optional[np.ndarray] = None, | ||
| mdd: Optional[np.ndarray] = None, | ||
| paa: Optional[np.ndarray] = None, | ||
| intensity_unit: str = "", | ||
| name: str = "", | ||
| ): |
There was a problem hiding this comment.
Update the __init__ signature with all attributes of the class:
- Use the exact same parameter names as the class attributes
- Add default values for every parameter. For mutable types (objects that can be manipulated), choose
None. Mutable default arguments are an anti-pattern! The default values will be set in the method body, see below. - Add type hints for each parameter. Import from the
typingmodule if suitable. - Order the parameters by "importance". Think about which parameters are the most essential for the class and place them first in the signature.
Pro-Tip: The CLIMADA documentation contains a Developer Guide with tips and best-practices for Python code development!
Comment on lines
+68
to
+89
| """Initialization. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| haz_type : str, optional | ||
| Hazard type acronym (e.g. 'TC'). | ||
| id : int or str, optional | ||
| id of the impact function. Exposures of the same type | ||
| will refer to the same impact function id. | ||
| intensity : np.array, optional | ||
| Intensity values. Defaults to empty array. | ||
| mdd : np.array, optional | ||
| Mean damage (impact) degree for each intensity (numbers | ||
| in [0,1]). Defaults to empty array. | ||
| paa : np.array, optional | ||
| Percentage of affected assets (exposures) for each | ||
| intensity (numbers in [0,1]). Defaults to empty array. | ||
| intensity_unit : str, optional | ||
| Unit of the intensity. | ||
| name : str, optional | ||
| Name of the ImpactFunc. | ||
| """ |
There was a problem hiding this comment.
Update the method docstring.
- If available, copy-paste the documentation of the class attributes (minding the correct order).
- Indicate which parameters are optional (these are the ones that do not have a default value).
- Explain the default values if they differ from the ones indicated in the signature (usually the ones with default value
None)
Comment on lines
+90
to
+97
| self.id = id | ||
| self.name = name | ||
| self.intensity_unit = intensity_unit | ||
| self.haz_type = haz_type | ||
| # Followng values defined for each intensity value | ||
| self.intensity = np.array([]) | ||
| self.mdd = np.array([]) | ||
| self.paa = np.array([]) | ||
| self.intensity = intensity if intensity is not None else np.array([]) | ||
| self.mdd = mdd if mdd is not None else np.array([]) | ||
| self.paa = paa if paa is not None else np.array([]) |
There was a problem hiding this comment.
Set the class attributes from the parameters.
- For immutable types:
self.param = param
- For mutable types:
self.param = param if param is not None else default
Comment on lines
192
to
+199
| inten_min, threshold, inten_max = intensity | ||
| impf.intensity = np.array([inten_min, threshold, threshold, inten_max]) | ||
| intensity = np.array([inten_min, threshold, threshold, inten_max]) | ||
| paa_min, paa_max = paa | ||
| impf.paa = np.array([paa_min, paa_min, paa_max, paa_max]) | ||
| paa = np.array([paa_min, paa_min, paa_max, paa_max]) | ||
| mdd_min, mdd_max = mdd | ||
| impf.mdd = np.array([mdd_min, mdd_min, mdd_max, mdd_max]) | ||
| mdd = np.array([mdd_min, mdd_min, mdd_max, mdd_max]) | ||
|
|
||
| return impf | ||
| return cls(id=impf_id, intensity=intensity, mdd=mdd, paa=paa) |
There was a problem hiding this comment.
Update the classmethods that initialize the class with different arguments than __init__.
- Instead of setting the class attributes, define variables and pass them to the class constructor
cls. - Use keyword arguments to "skip over" arguments that should remain defaulted.
- Prefer using the new constructor signature over setting the class arguments (which would still be valid)
Comment on lines
+32
to
+35
| intensity = np.arange(0, 100, 10) | ||
| paa = np.arange(0, 1, 0.1) | ||
| mdd = np.arange(0, 1, 0.1) | ||
| imp_fun = ImpactFunc(intensity=intensity, paa=paa, mdd=mdd) |
There was a problem hiding this comment.
Update the tests to use the new constructor signature.
Comment on lines
+83
to
+92
| intensity = np.linspace(0, 150, num=100) | ||
| mdd = np.repeat(1, len(intensity)) | ||
| paa = np.array([sigmoid_function(v, G, v_half, vmin, k) | ||
| for v in intensity]) | ||
| imp_fun = ImpactFunc(haz_type='TC', | ||
| id=_id, | ||
| intensity_unit='m/s', | ||
| intensity=intensity, | ||
| mdd=mdd, | ||
| paa=paa) |
There was a problem hiding this comment.
Update docs and tutorials explaining the usage of the class
peanutfun
changed the title
11 tasks
|
🙌 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this PR:
ImpactFunc.__init__to set all class attributes during initializationImpactFuncclassmethodsPR Author Checklist
develop)PR Reviewer Checklist