Repository: BAMWelDX/weldx

Issue:
negative parameters for welding groove shapes
It is currently possible to create groove types with negative parameter values which has no technical application

We should probably add a quick check to verify

Discussion:
pep8speaks: Hello @CagtayFabry! Thanks for opening this PR. 



* In the file [`weldx/welding/groove/iso_9692_1.py`](https://github.com/BAMWelDX/weldx/blob/ed44b0c4a361108a4e18e1cf0704ad3642a68034/weldx/welding/groove/iso_9692_1.py):

> [Line 86:21](https://github.com/BAMWelDX/weldx/blob/ed44b0c4a361108a4e18e1cf0704ad3642a68034/weldx/welding/groove/iso_9692_1.py#L86): [E225](https://duckduckgo.com/?q=pep8%20E225) missing whitespace around operator
> [Line 90:5](https://github.com/BAMWelDX/weldx/blob/ed44b0c4a361108a4e18e1cf0704ad3642a68034/weldx/welding/groove/iso_9692_1.py#L90): [E303](https://duckduckgo.com/?q=pep8%20E303) too many blank lines (2)

Do see the [Hitchhiker's guide to code style](https://goo.gl/hqbW4r)
codecov[bot]: # [Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/306?src=pr&el=h1) Report
> Merging [#306](https://codecov.io/gh/BAMWelDX/weldx/pull/306?src=pr&el=desc) (ed44b0c) into [master](https://codecov.io/gh/BAMWelDX/weldx/commit/3e1479269d770c3d076086a5ebc6c94607d059d8?el=desc) (3e14792) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head ed44b0c differs from pull request most recent head 3b0e4d0. Consider uploading reports for the commit 3b0e4d0 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/BAMWelDX/weldx/pull/306/graphs/tree.svg?width=650&height=150&src=pr&token=wdof1qQTsn)](https://codecov.io/gh/BAMWelDX/weldx/pull/306?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #306   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files          77       77           
  Lines        4135     4139    +4     
=======================================
+ Hits         4120     4124    +4     
  Misses         15       15           
```


| [Impacted Files](https://codecov.io/gh/BAMWelDX/weldx/pull/306?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [weldx/welding/groove/iso\_9692\_1.py](https://codecov.io/gh/BAMWelDX/weldx/pull/306/diff?src=pr&el=tree#diff-d2VsZHgvd2VsZGluZy9ncm9vdmUvaXNvXzk2OTJfMS5weQ==) | `99.51% <100.00%> (+<0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/306?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/306?src=pr&el=footer). Last update [3e14792...3b0e4d0](https://codecov.io/gh/BAMWelDX/weldx/pull/306?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

CagtayFabry: I think the  AppVeyor build because I accidentally pushed the branch here isntead of my fork
vhirtham: > I think the AppVeyor build because I accidentally pushed the branch here isntead of my fork

Appveyor fails because there is a merge conflict with the master branch (changelog) ;) 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4245aaa..338f58e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,9 @@
 - add validators to `rotation-1.0.0.yaml`
   & `gas_component-1.0.0.yaml` [[#303]](https://github.com/BAMWelDX/weldx/pull/303)
 
+### fixes
+
+- prevent creation of `IsoBaseGroove` with negative parameters [[#306]](https://github.com/BAMWelDX/weldx/pull/306)
 
 ## 0.3.1 (21.03.2021)
 
diff --git a/weldx/welding/groove/iso_9692_1.py b/weldx/welding/groove/iso_9692_1.py
index c3264b9..d70c5f2 100644
--- a/weldx/welding/groove/iso_9692_1.py
+++ b/weldx/welding/groove/iso_9692_1.py
@@ -81,6 +81,12 @@ class IsoBaseGroove(metaclass=abc.ABCMeta):
     _AREA_RASTER_WIDTH = 0.1
     """steers the area approximation of the groove in ~cross_sect_area."""
 
+    def __post_init__(self):
+        """Make sure all parameters are valid after class init."""
+        for key, value in self.parameters().items():
+            if value < 0.0:
+                raise ValueError(f"Invalid value for parameter {key}={value:~}")
+
     def parameters(self):
         """Return groove parameters as dictionary of quantities."""
         return {k: v for k, v in self.__dict__.items() if isinstance(v, pint.Quantity)}
@@ -1719,6 +1725,11 @@ def get_groove(
                    root_face=Q_(3, "mm"),
                    root_gap=Q_(1, "mm"))
 
+    Raises
+    ------
+    ValueError
+        When passing negative parameter values.
+
     Notes
     -----
     Each groove type has a different set of attributes which are required. Only


Test patch:
diff --git a/tests/asdf_tests/test_asdf_groove.py b/tests/asdf_tests/test_asdf_groove.py
index e883583..29902a2 100644
--- a/tests/asdf_tests/test_asdf_groove.py
+++ b/tests/asdf_tests/test_asdf_groove.py
@@ -94,6 +94,16 @@ def test_asdf_groove_exceptions():
             code_number="6.1.1",
         ).to_profile()
 
+    # negative parameter value
+    with pytest.raises(ValueError):
+        get_groove(
+            groove_type="VGroove",
+            workpiece_thickness=Q_(9, "mm"),
+            groove_angle=Q_(50, "deg"),
+            root_face=Q_(-4, "mm"),
+            root_gap=Q_(2, "mm"),
+        )
+
 
 @pytest.mark.parametrize("groove", test_params.values(), ids=test_params.keys())
 def test_cross_section(groove):  # noqa