-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Zero length string now handled for the Depth property #555
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the updated code using the sample dwelling and script which is referenced in the original issue. Looks like it's working well now with these changes
@BHoMBot check compliance |
@FraserGreenroyd to confirm, the following checks are now queued:
There are 260 requests in the queue ahead of you. |
@BHoMBot check compliance |
@jamesramsden-bh to confirm, the following actions are now queued:
|
@BHoMBot check compliance |
@jamesramsden-bh to confirm, the following actions are now queued:
There are 6 requests in the queue ahead of you. |
@BHoMBot check required |
@jamesramsden-bh to confirm, the following actions are now queued:
There are 2 requests in the queue ahead of you. |
The check |
The check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's working, the data from XML files is coming through as expected.
The check |
@BHoMBot check ready-to-merge |
@jamesramsden-bh to confirm, the following actions are now queued:
|
@BHoMBot check ready-to-merge |
@jamesramsden-bh to confirm, the following actions are now queued:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on a call with @jamesramsden-bh and @CKBoulter just now, agreement to add a Query
method to query the double depth to support numerical workflows.
public static double Depth(this Markup markup)
{
try
{
return System.Convert.ToDouble(markup.XMLDepth);
}
catch(Exception e)
{
BH.Engine.Base.Compute.RecordError("Could not convert XMLDepth to numerical depth.");
return -1;
}
}
@rboulton-BH @EKAdebo is a return of -1
for a failed depth suitable here?
@CKBoulter I have queued up your request to fix the |
@CKBoulter I'm sorry, but I cannot take that instruction from you. As this action would modify this Pull Request, this instruction can only come via an authorised user, per our Code of Conduct for Bots |
@BHoMBot fix project file ref. 9695000940 |
@FraserGreenroyd I have queued up your request to fix the |
@FraserGreenroyd I am now going to fix the project compliance in accordance with the annotations previously made. |
@FraserGreenroyd to confirm I have now resolved the project compliance issues and pushed a commit to this Pull Request. |
@BHoMBot check compliance |
@FraserGreenroyd to confirm, the following actions are now queued:
There are 4 requests in the queue ahead of you. |
@BHoMBot check required |
@FraserGreenroyd to confirm, the following actions are now queued:
There are 3 requests in the queue ahead of you. |
The check |
The check |
The check |
The check |
@BHoMBot check null-handling |
@FraserGreenroyd to confirm, the following actions are now queued:
|
@FraserGreenroyd to confirm, the following actions are now queued:
There are 1 requests in the queue ahead of you. |
The check |
The check |
The check |
The check |
The check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionality remains the same following @EKAdebo review, new method looks good and works well. Happy to merge to alpha.
@BHoMBot check ready-to-merge |
@FraserGreenroyd to confirm, the following actions are now queued:
There are 55 requests in the queue ahead of you. |
Closes #554
Test files
Try different values of 'depth' in the XML file. Verify the value of 'Depth' in the exploded markup.
test.zip
Changelog
Additional comments