Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updating result extraction to be using result requests #129

Merged
merged 10 commits into from
Nov 28, 2019
Merged

Updating result extraction to be using result requests #129

merged 10 commits into from
Nov 28, 2019

Conversation

IsakNaslundBh
Copy link
Contributor

@IsakNaslundBh IsakNaslundBh commented Nov 22, 2019 •
edited
Loading

Issues addressed by this PR

Closes #126

Test files

https://burohappold.sharepoint.com/:f:/r/sites/BHoM/02_Current/12_Scripts/01_Test%20Scripts/GSA_Toolkit/GSA_Toolkit-Issue126-ReadResultUpdate?csf=1&e=rEcEy2

Code will work without it, but to be able to view deformed shape, BHoM/BHoM_Engine#1350 is needed

Changelog

  • Update result extraction to work with ResultRequests. Support for BarResultRequest, NodeResultRequest and GlobalResultRequest added.
  • For BarResult this now means better control over how results are extracted.
  • Outputting BarDisplacements instead of BarDeformations.
  • Updated warning and error messaging

Additional comments

@IsakNaslundBh IsakNaslundBh added type:feature New capability or enhancement status:WIP PR in progress and still in draft, not ready for formal review labels Nov 22, 2019
@IsakNaslundBh IsakNaslundBh added this to the BHoM 3.0 β MVP milestone Nov 22, 2019
@IsakNaslundBh IsakNaslundBh self-assigned this Nov 22, 2019
@alelom
Copy link
Member

alelom commented Nov 26, 2019

@IsakNaslundBh is this ready for review?

@alelom alelom self-requested a review November 26, 2019 15:01
@IsakNaslundBh IsakNaslundBh marked this pull request as ready for review November 26, 2019 15:07
@IsakNaslundBh
Copy link
Contributor Author

@alelom Yes, think it should be. Will try to put together a testfile, unless you are able to?

@IsakNaslundBh IsakNaslundBh removed the status:WIP PR in progress and still in draft, not ready for formal review label Nov 26, 2019
alelom
alelom approved these changes Nov 27, 2019
View reviewed changes
Copy link
Member

@alelom alelom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine. Code LGTM. I cross-checked result extraction with GSA values for Dynamic Results, Global Reactions and Node Results.

GSA_Adapter/GSA_Adapter.csproj
Comment on lines +105 to +106
<Compile Include="CRUD\ReadResultsElements.cs" />
<Compile Include="CRUD\ReadResultsGlobal.cs" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate this new subdivision of the methods instead of having everything in ReadResults

@IsakNaslundBh IsakNaslundBh merged commit c37c135 into master Nov 28, 2019
@IsakNaslundBh IsakNaslundBh deleted the GSA_Toolkit-#126-ResultExtractionUsingResultRequests branch November 28, 2019 14:02
This was referenced Dec 19, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alelom alelom alelom approved these changes

Assignees

@IsakNaslundBh IsakNaslundBh

Labels
type:feature New capability or enhancement
Projects
None yet
Milestone
BHoM 3.0 β MVP
Development

Successfully merging this pull request may close these issues.

GSA_Toolkit: Update Result Extraction to make use of new ResultRequests
2 participants
@IsakNaslundBh @alelom