GSAGH-497 - GsaGH creates releases opposite to what is defined using 'Edit element/member' component #721

SandeepArup · 2024-11-05T11:05:55Z

No description provided.

…'Edit element/member' component

psarras · 2024-11-06T09:13:28Z

GsaGH/Helpers/GH/ReplaceParam.cs

why we need this update?

Update is required as opening file will have info of GsaBool6Parameter but that does not know how to parse "FFFRRR" as those cast has been removed from GsaBool6Parameter.

psarras · 2024-11-06T09:15:56Z

GsaGH/Helpers/StringExtension.cs

+        } else if (mystring == "release" || mystring == "released" || mystring == "hinge"
+          || mystring == "hinged" || mystring == "charnier") {


wrap with a method or two. IsHinged(), IsReleased()

psarras · 2024-11-06T09:17:10Z

GsaGH/Helpers/StringExtension.cs

+      if (FindString(booleanString, "x☐") || FindString(booleanString, "x✓")
+          || FindString(booleanString, "y☐") || FindString(booleanString, "y✓")
+          || FindString(booleanString, "z☐") || FindString(booleanString, "z✓")
+          || FindString(booleanString, "xx☐") || FindString(booleanString, "xx✓")
+          || FindString(booleanString, "yy☐") || FindString(booleanString, "yy✓")
+          || FindString(booleanString, "zz☐") || FindString(booleanString, "zz✓")) {
+
+        input.X = FindString(booleanString, "x✓");
+        input.Y = FindString(booleanString, "y✓");
+        input.Z = FindString(booleanString, "z✓");
+        input.Xx = FindString(booleanString, "xx✓");
+        input.Yy = FindString(booleanString, "yy✓");
+        input.Zz = FindString(booleanString, "zz✓");
+        return true;


hmm why do we check this? Can we not check the True / False values instead?

GsaGH/Helpers/StringExtension.cs

psarras · 2024-11-06T09:21:32Z

GsaGH/Parameters/1_Properties/GsaReleaseParameter.cs

Did we not said to create a common ancestor for GsaReleaseParameter and GsaRestrainParameter?

Is this what you are expecting i.e. Parameter inherited from GsaBool6Goo? https://github.com/arup-group/GSA-Grasshopper/pull/721/files#diff-2146e40799484c09b42c0927d6f7fcb4a9490500c84cc3c0b360a3c019ecd183

…ameter

…GsaBool6Parameter

…release

SandeepArup · 2024-11-14T06:46:59Z

CI

DominikaLos · 2024-11-14T10:04:32Z

GsaGH/Helpers/GH/ReplaceParam.cs

@@ -36,5 +38,63 @@ public static string UnsupportedValue(GH_ObjectWrapper ghTypeWrapper) {
      type = type.Replace("Goo", string.Empty);
      return type;
    }
+
+    public static void UpdateReleaseBool6Parameter(this GH_ComponentParamServer parameters) {


why not 2 methods? one for input and one for output

DominikaLos · 2024-11-14T10:04:46Z

GsaGH/Helpers/GH/ReplaceParam.cs

+      }
+    }
+
+    public static void UpdateRestrainedBool6Parameter(this GH_ComponentParamServer parameters) {


DominikaLos · 2024-11-14T10:07:39Z

GsaGHTests/1_BaseParameters/1_Properties/GsaBool6Test.cs

@@ -83,5 +88,67 @@ public void DuplicateTest(bool x, bool y, bool z, bool xx, bool yy, bool zz) {
      Assert.Equal(yy, original.Yy);
      Assert.Equal(zz, original.Zz);
    }
+
+    [Fact]
+    public void ReleaseParameterInfoIsCorrect() {


split to include one assert per test

DominikaLos · 2024-11-14T10:08:16Z

GsaGHTests/2_GooWrappers/GsaBool6GooTest.cs

+
+      //restraint is the opposite of release
+      output = StringExtension.ParseRestrain(text);
+      Assert.Equal(!releaseBool6, output);


DominikaLos · 2024-11-14T10:08:27Z

GsaGHTests/3_Components/2_Geometry/EditElement1dTests.cs

@@ -30,6 +30,17 @@ public static GH_OasysComponent ComponentMother() {
      return comp;
    }

+    internal static void CompareRelease(GsaBool6 input, GsaBool6 output) {


DominikaLos · 2024-11-14T10:14:09Z

GsaGHTests/1_BaseParameters/1_Properties/GsaBool6Test.cs

@@ -83,5 +88,67 @@ public void DuplicateTest(bool x, bool y, bool z, bool xx, bool yy, bool zz) {
      Assert.Equal(yy, original.Yy);
      Assert.Equal(zz, original.Zz);
    }
+
+    [Fact]
+    public void ReleaseParameterInfoIsCorrect() {


one assert per test

DominikaLos · 2024-11-14T10:14:32Z

GsaGHTests/3_Components/2_Geometry/EditElement1dTests.cs

+    internal static void CompareRelease(GsaBool6 input, GsaBool6 output) {
+      Assert.Equal(input.ToString(), output.ToString());
+      Assert.Equal(input.ToString(), output.ToString());
+      Assert.Equal(input.X, output.X);


psarras · 2024-11-14T13:13:02Z

GsaGH/Components/2_Geometry/EditNode.cs

@@ -59,7 +59,7 @@ public override bool Read(GH_IReader reader) {
        Params.ReplaceInputParameter(new GsaSpringPropertyParameter(), 7, true);
        Params.ReplaceOutputParameter(new GsaSpringPropertyParameter(), 7);
      }
-
+      Params.UpdateRestrainedBool6Parameter();


is this for upgrading existing components with Bool6Params?

codecov · 2024-11-15T11:47:17Z

Codecov Report

Attention: Patch coverage is 98.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 90.2%. Comparing base (83764a7) to head (5aec6be).

Files with missing lines	Patch %	Lines
GsaGH/Helpers/StringExtension.cs	97.2%	1 Missing and 1 partial ⚠️
GsaGH/Parameters/1_Properties/GsaBool6.cs	90.9%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main    #721    +/-   ##
======================================
  Coverage   90.1%   90.2%            
======================================
  Files        520     522     +2     
  Lines      39296   39408   +112     
  Branches    4902    4914    +12     
======================================
+ Hits       35441   35553   +112     
+ Misses      2582    2580     -2     
- Partials    1273    1275     +2

Files with missing lines	Coverage Δ
GsaGH/Components/2_Geometry/CreateSupport.cs	`98.7% <100.0%> (+<0.1%)`	⬆️
GsaGH/Components/2_Geometry/Edit1dElement.cs	`96.4% <100.0%> (+<0.1%)`	⬆️
GsaGH/Components/2_Geometry/Edit1dMember.cs	`94.9% <100.0%> (+<0.1%)`	⬆️
GsaGH/Components/2_Geometry/EditNode.cs	`87.4% <100.0%> (+<0.1%)`	⬆️
GsaGH/Helpers/GH/ReplaceParam.cs	`100.0% <100.0%> (ø)`
GsaGH/Parameters/1_Properties/GsaBool6Parameter.cs	`85.0% <ø> (-8.1%)`	⬇️
...aGH/Parameters/1_Properties/GsaReleaseParameter.cs	`100.0% <100.0%> (ø)`
.../Parameters/1_Properties/GsaRestrainedParameter.cs	`100.0% <100.0%> (ø)`
GsaGH/Properties/Resources.Designer.cs	`90.6% <100.0%> (+<0.1%)`	⬆️
GsaGH/Helpers/StringExtension.cs	`97.6% <97.2%> (-2.4%)`	⬇️
... and 1 more

SandeepArup added 3 commits November 5, 2024 16:35

GSAGH-497 - GsaGH creates releases opposite to what is defined using …

30d643a

…'Edit element/member' component

GSAGH-497 - update test

b302f35

GSAGH-497: update test

c4bf6ee

psarras requested changes Nov 6, 2024

View reviewed changes

SandeepArup added 5 commits November 6, 2024 17:33

GSAGH-497: common ancestor for GsaReleaseParameter and GsaRestrainPar…

2e1e0cd

…ameter

GSAGH-497: Inherit GsaReleaseParameter and GsaRestrainParameter from …

1447473

…GsaBool6Parameter

GSAGH-497: wrong end condition calculation

5471f08

GSAGH-497: remove unused function

553d70d

GSAGH-497 - test to increase coverage

4850f6e

SandeepArup marked this pull request as ready for review November 7, 2024 05:36

psarras and others added 5 commits November 12, 2024 13:02

refactor: reworked the way we do the casting so we can reduce complexity

051d921

fix: removed equals on GsaBool6Goo

39e7d14

test: added Runtime message to satisfy the tests

d3471ee

test: more tests for GsaBool6 and removed unecessary methods

9073fdc

GSAGH-497: update integration test to check restraint is opposite to …

f507018

…release

SandeepArup and others added 2 commits November 14, 2024 12:17

Merge branch 'main' into task/GSAGH-497-new-parameter

782f83f

GSAGH-497: release and restraint parameter test added

5aec6be

DominikaLos requested changes Nov 14, 2024

View reviewed changes

psarras approved these changes Nov 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GSAGH-497 - GsaGH creates releases opposite to what is defined using 'Edit element/member' component #721

GSAGH-497 - GsaGH creates releases opposite to what is defined using 'Edit element/member' component #721

SandeepArup commented Nov 5, 2024

psarras Nov 6, 2024

SandeepArup Nov 6, 2024

psarras Nov 6, 2024

psarras Nov 6, 2024

psarras Nov 6, 2024

SandeepArup Nov 6, 2024

SandeepArup commented Nov 14, 2024

DominikaLos Nov 14, 2024

DominikaLos Nov 14, 2024

DominikaLos Nov 14, 2024

DominikaLos Nov 14, 2024

DominikaLos Nov 14, 2024

DominikaLos Nov 14, 2024

DominikaLos Nov 14, 2024

psarras Nov 14, 2024

codecov bot commented Nov 15, 2024

		} else if (mystring == "release" \|\| mystring == "released" \|\| mystring == "hinge"
		\|\| mystring == "hinged" \|\| mystring == "charnier") {

GSAGH-497 - GsaGH creates releases opposite to what is defined using 'Edit element/member' component #721

Are you sure you want to change the base?

GSAGH-497 - GsaGH creates releases opposite to what is defined using 'Edit element/member' component #721

Conversation

SandeepArup commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SandeepArup commented Nov 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 15, 2024

Codecov Report