Implement STEP export #11

simonmoreau · 2017-11-14T19:56:13Z

This pull request implements export functions in the IFC classes to create an IFC (STEP) file.
It adds the method STEPValue(ref Dictionary<Guid, int> indexDictionnary), which need an indexDictionnary to keep track of the relation between STEP index and Guid. This method gives for each IFC class its corresponding STEP value in the file (whether an index or an inline value).
This pull request follow another pull request in IFC-dotnet

Embedding STEP export capabilities into the generated classes

ikeough · 2017-11-21T14:59:38Z

@simonmoreau I saw your tweet about making a PR to one of my repos and I hadn't recognized that this was the one. Perhaps a setting with my notifications :( In any case, I apologize for not having seen this sooner. I'll take a look and get back to. Might take a couple of days with the Holidays here in the US.

ikeough · 2017-11-21T15:01:46Z

@simonmoreau You should know as well that I am merging IFC-dotnet into IFC-gen. Working back and forth between the two repositories was causing unnecessary overhead. So your PR in IFC-dotnet will have to be applied here (or I might do it if the change seems reasonable). I will be making the merge soon and I'll update you with how this relates to your PRs.

ikeough

Thanks very much for this submission @simonmoreau! I've highlighted a number of improvements, mostly syntactical. There will most likely be another round of review when these changes are picked up.

ikeough · 2017-11-22T14:08:52Z

src/IFC-gen/csharp/ExpressListener.cs

 	{
 		[JsonProperty(""id"")]
-		public Guid Id{get;}
+		public Guid Id{get;set;}


This was intentionally left without a setter. After an object of type IfcBase is constructed, we do not want this value to be changed. Is there a way that we can set this value without giving it a public setter? Could we make this property wrap a private id field and use one of the STEP reading methods to set that instead?

ikeough · 2017-11-22T14:12:00Z

src/IFC-gen/csharp/ExpressListener.cs

-			throw new NotImplementedException();
-		}
-	}
+        public virtual string STEPParameters(ref Dictionary<Guid, int> indexDictionnary)


Dictionary...spelling aside, I think we could just call this indexMap or simply indices.

ikeough · 2017-11-22T14:19:22Z

src/IFC-gen/csharp/TypeData.cs

+            string prop = "";
+            if (Type.EndsWith("Enum") | Type == "bool" | Type == "int" | Type == "double")
+            {
+                prop = $"\t\t\tparameters.Add({Name}.STEPValue(ref indexDictionnary));\n";


I've taken to not adding newlines in formatted strings. Usually, ToSTEPString will be called when calling StringBuilder.AppendLine(...), and adding the newlines explicitly to the string can cause duplication of newlines.

ikeough · 2017-11-22T14:20:58Z

src/IFC-gen/csharp/TypeData.cs

 		}
 	}

-	/// <summary>


What happened with TypeData? Was a tab inserted or removed?

ikeough · 2017-11-22T14:23:59Z

src/IFC-gen/csharp/ExpressListener.cs

+        {
+            if (indexDictionnary.ContainsKey(Id))
+            {
+                return ""#"" + indexDictionnary[Id].ToString();


IFC-gen uses new-age c# string replacement. This could be $"return {indexDictionary[Id].ToString()}".

ikeough · 2017-11-22T14:29:15Z

src/IFC-gen/csharp/ExpressListener.cs

+
+        public virtual string ToSTEP(ref Dictionary<Guid, int> indexDictionnary)
+        {
+            return string.Format(""{0} = {1}({2});\r\n"",


See comments elsewhere re: newlines and string replacement.

ikeough · 2017-11-22T14:34:36Z

src/IFC-gen/csharp/ExpressListener.cs

+                this.STEPParameters(ref indexDictionnary));
+        }
+
+        public virtual string STEPValue(ref Dictionary<Guid, int> indexDictionnary)


We could simplify this method by assuming that the Dictionary<Guid,int> will be maintained outside this method. This would change the signature to STEPValue(int index).

ikeough · 2017-11-22T14:48:24Z

src/IFC-gen/csharp/TypeData.cs

+                }
+                else
+                {
+                    string stepValue =


I think these can be simplified to just return $@"...".

ikeough · 2017-11-22T14:55:14Z

@simonmoreau I've merged the changes which I mentioned above. I apologize that this will add some extra work for you to figure out how to adapt your PR. I'm hoping that these organizational changes to the repo will settle down soon.

simonmoreau · 2017-11-22T15:50:18Z

@ikeough Thanks for the review ! I will edit my PR and add the IFC-dotnet one in this repo.

Build in windows

Support for non-US culture

Edit IFC-dotnet-test.csproj to be able to debug test

Add Extension to export simple type to STEP

Implement ToSTEP(Dictionary<Guid, int> indexMap) to convert an IfcBase into its STEP string Implement ToStepValue(Dictionary<Guid, int> indexMap) to convert an Ifc entity into its STEP index, or its STEP value as a string Implement GetStepParameters(Dictionary<Guid, int> indexMap) to get all attributes as either STEP index or STEP value

ikeough · 2017-12-11T07:58:41Z

@simonmoreau I haven't forgotten about this. I've made some incompatible changes since your updates and rather than ask to you update this PR yet again, I'm merging your changes manually. Hopefully this will be done soon.

simonmoreau · 2017-12-12T09:59:36Z

@ikeough I also kept working on it, if you want, I can edit the PR again to integrate these changes

ikeough · 2017-12-12T14:01:47Z

Thanks, but I’m almost there. There was some significant challenges with the STEP writer writing STEP files that were invalid, which I’ve almost ironed out. I should be able to merge this code today.

ikeough · 2017-12-12T17:42:14Z

@simonmoreau I've merged your contributions and my updates to align them with master. Thanks very much for this submission. You will note that I did a couple things to your code:

Replaced string continuations like += "some other string" with multi-line interpolated strings. This is the default for code "templates" in the C# code. One day, I might even move these strings to separate template files but I couldn't find a good C# string template library that worked with dotnet core.
Moved logic that you had written into TypeData into CSharpLanguageGenerator. One of the overlapping changes that I had made was to make TypeData target language agnostic.
Fixed several errors in the generated STEP files, like lines not ending with ;.

The STEP export is still broken due to IFCType<T> instances being stored in the the Document and writing their own lines to the STEP file. I've opened https://github.com/ikeough/IFC-gen/issues/14 to track this.

simonmoreau · 2017-12-13T12:54:30Z

Great! Thank you for your help!

simonmoreau added 2 commits November 14, 2017 12:23

Read-Write Guid

2e767f7

STEP Export

0cc22a5

Embedding STEP export capabilities into the generated classes

ikeough requested changes Nov 22, 2017

View reviewed changes

simonmoreau added 12 commits November 22, 2017 21:51

Add make.bat

1ec0dde

Build in windows

Edit make.bat

92c66a7

Invariant Culture

6d4fc54

Support for non-US culture

Merge remote-tracking branch 'ikeough/master'

d30c38f

Correct path to .csproj

d64a746

Debug test

f57030b

Edit IFC-dotnet-test.csproj to be able to debug test

Merge remote-tracking branch 'simonmoreau/master'

3927f9a

Add ToStep() in Model.cs

39374ee

Add Extension to export simple type to STEP

Edit Make.bat

7128c57

Merge remote-tracking branch 'ikeough/master'

d18f258

Add Step Export test

f48558e

ikeough merged commit ec4a8e1 into hypar-io:master Dec 12, 2017

349516385 mentioned this pull request Feb 23, 2020

Is "TypeRef" in "STEP.g4" right? #59

Open

Implement STEP export #11

Implement STEP export #11

Uh oh!

Conversation

simonmoreau commented Nov 14, 2017

Uh oh!

ikeough commented Nov 21, 2017

Uh oh!

ikeough commented Nov 21, 2017

Uh oh!

ikeough left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ikeough commented Nov 22, 2017

Uh oh!

simonmoreau commented Nov 22, 2017

Uh oh!

ikeough commented Dec 11, 2017

Uh oh!

simonmoreau commented Dec 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ikeough commented Dec 12, 2017

Uh oh!

ikeough commented Dec 12, 2017

Uh oh!

simonmoreau commented Dec 13, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

simonmoreau commented Dec 12, 2017 •

edited

Loading