-
Notifications
You must be signed in to change notification settings - Fork 1
Extend BED-JSON to include everything from LBNL-JSON #352
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
Conversation
059859d to
22d1480
Compare
|
@simon-wacker Thank you for your comments! I have included them as good as I was able to. |
|
@simon-wacker @christoph-maurer What is the status of this feature? Do you need anything from me to finish it? |
|
@danielmcquillen Please check if this pull request includes all data which IGSDB is currently sharing via REST. Please also check that the data within the key |
danielmcquillen
left a comment
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.
@christoph-maurer I clicked approve but I don't see anything in the changed files that relates directly to the IGSDB v2 API. Can you clarify exactly what you want me to validate?
@danielmcquillen Currently, the REST endpoint of IGSDB returns more data than the GraphQL endpoint. This pull request shall extend
Can you please verify that this is true? igsdbExampleClearlite-4_250312.json is an example of a current optical dataset from the GraphQL endpoint of IGSDB. igsdbExampleClearlite-4_extendedBED.json is an example how optical datasets from the GraphQL endpoint of IGSDB could look like in the future. The key "igsdb" adds the data which is currently only available via the REST endpoint of IGSDB. We can discuss it in a web conference if you like. |
|
@christoph-maurer It's a little hard to compare manually since it's camel case by convention in GraphQL and our API uses snake case. However, trying to control for that I used DeepDiff to compare what you have in your new 'igsdb' field to the standard result for clr-3 in the API. I see the following fields in our data but not in yours. Maybe this is ok based on what you're trying to do? I also see that you have geometric properties at same level as physical properties, but we have moved it to a child of physical properties:
I got this data by creating a quick test case to compare these structures, as it's hard to manually check. |
292318e to
355b18e
Compare
These values are handled by database.grapqhql. It would be redundant to return them in the JSON dataset, too.
These values are not part of "serverSpecific", but part of "data". "data" contains values which are already shared as BED-JSON. Currently, the solar, visible and infrared integrated spectral averages are shared as BED-JSON. opticalData.json allows to share all values of
With b52de6c, we have moved "geometricProperties" to "physicalProperties".
@danielmcquillen Thank you for this very helpful automated check! Do we agree that all keys of the current LBNL-JSON can be shared in the extended BED-JSON of this pull request? And do you want any changes regarding the allowed values of the keys? I often used "numberOrNull" or "stringOrNull" based on the example dataset. |
|
@christoph-maurer I can discuss the status of this PR with @RDmitchell and @jcjonsson at this week's meeting. However, just to clarify the overall gist of this PR:
Is that right? I'm not sure what you mean in this statement: "If we can agree to extend BED-JSON and to use only the extended BED-JSON in the future, @simon-wacker and @christoph-maurer can try to help to update IGSDB accordingly." ... can you clarify what you mean here so it can be discussed in the meeting? |
@danielmcquillen Yes.
Currently, the IGSDB has a REST-API and a GraphQL-API. For the example clearlite-4, the REST-API returns https://igsdb-v2.herokuapp.com/api/v2/products/clearlite-4?format=json and the GraphQL-API returns https://igsdb-v2.herokuapp.com/api/v2/products/clearlite-4?format=bed-optical-json . This can confuse software companies who want to use the data. I understood Charlie that IGSDB should use BED-JSON in the future. However, https://igsdb-v2.herokuapp.com/api/v2/products/clearlite-4?format=json contains many keys which have not been available in BED-JSON yet. Therefore, I promised Charlie that I make a proposal how BED-JSON could be extended to cover all key-value pairs which the REST-API currently covers. When we agree on the extension and implement it in IGSDB, the REST-API and the GraphQL-API will return the same result, for example https://github.com/building-envelope-data/api/blob/41bcde4784612baeec77b1f2952ef0e403e558ce/tests/valid/opticalData/igsdbExampleClearlite-4_extendedBED.json . If you have any questions, please don't hesitate to let me know! We could also arrange a web meeting. Thanks for discussing it with @RDmitchell and @jcjonsson ! |
|
@christoph-maurer -- Charlie is out of the office this week and next so Jacob and I won't have time to talk to him about this until then. |
|
Thanks for the update, @RDmitchell ! |
|
@RDmitchell I propose that we extend the BED-JSON by merging this branch and that we wait with implementing it in IGSDB until we have enough resources. What do you think? |
|
@christoph-maurer -- we are not going to change the base API response format; however, if the user uses the format BED-JSON modifier, we can shape that data however you want. So we are going to leave the current IGSDB API response format as is. We talked this over with Charlie and he agreed. |
Thank you, @RDmitchell ! |
41bcde4 to
5919474
Compare
…le to validate the extension of the BED-JSON.

Currently, the IGSDB provides optical data as BED-JSON, but also as LBNL-JSON. This can confuse software developers who want to use the data.
LBNL and Fraunhofer ISE have therefore agreed to extend the existing BED-JSON, especially opticalData.json. This extension covers all data which is currently covered by LBNL-JSON.