search

NREL / BuildingMOTIF

Building Metadata OnTology Interoperability Framework (BuildingMOTIF)
https://buildingmotif.readthedocs.io/
Other
42 stars 6 forks source link

Add Application Explorer #292

Closed haneslinger closed 7 months ago

haneslinger commented 8 months ago
image
gtfierro commented 8 months ago

This looks great! My one suggestion is the bolded text in the explorer should be the RDFS label, and the URI should go underneath. The URI itself is mostly internal information and isn't that important to display to the user.

Could we also sort the target node field alphabetically so it is easier to find things?

Thanks!

haneslinger commented 8 months ago

My one suggestion is the bolded text in the explorer should be the RDFS label, and the URI should go underneath. You got it.

Could we also sort the target node field alphabetically so it is easier to find things? I'm almost certain it is, no?

haneslinger commented 8 months ago

@MatthewSteen how do I appease github actions?

TShapinsky commented 8 months ago

I'm working on a pr to fix them. It's mostly Python 3.12 stuff

On Mon, Oct 30, 2023, 13:22 Hannah Eslinger @.***> wrote:

@MatthewSteen https://github.com/MatthewSteen how do I appease github actions?

— Reply to this email directly, view it on GitHub https://github.com/NREL/BuildingMOTIF/pull/292#issuecomment-1785892372, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDNI6LWNIOIVY3Z4VPGVT3YB747BAVCNFSM6AAAAAA6R2546GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBVHA4TEMZXGI . You are receiving this because your review was requested.Message ID: @.***>

TShapinsky commented 8 months ago

@haneslinger If you want to bring this branch up to date with develop I think it's good to merge. Or are you holding off after recent discussions?

haneslinger commented 7 months ago

lets merge babeeee

TShapinsky commented 7 months ago

lets merge babeeee

@haneslinger are you able to merge it? gh is throwing an error for me trying to do it in the browser

haneslinger commented 7 months ago

At time on post, github's having issues

image
haneslinger commented 7 months ago

@gtfierro, the last commit made runs super slow. I did some testing and it's optional in the query. why?

gtfierro commented 7 months ago

Weird! Do you have a repro I could use to investigate the performance regression? Or an action I can take in the UI?

We can factor the OPTIONAL clause out of the query if necessary and put that into the for loop. Something like graph.value(entiity, RDFS.label) will fetch the label if one exists on the entity

haneslinger commented 7 months ago 
curl 'http://localhost:5000/libraries/shapes' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Gecko/20100101 Firefox/115.0' -H 'Accept: application/json, text/plain, */*' -H 'Accept-Language: en-US,en;q=0.5' -H 'Accept-Encoding: gzip, deflate, br' -H 'Origin: http://localhost:4200' -H 'Connection: keep-alive' -H 'Referer: http://localhost:4200/' -H 'Sec-Fetch-Dest: empty' -H 'Sec-Fetch-Mode: cors' -H 'Sec-Fetch-Site: same-site' -H 'Pragma: no-cache' -H 'Cache-Control: no-cache'

there will need to be libraries in your db to get the performance change, obi.

gtfierro commented 7 months ago

Thanks for the repro! That was super helpful

I pushed a small fix which factors out the SPARQL query. The direct property lookups (graph.value(...)) are much faster than having the OPTIONAL in the SPARQL query. On my machine the execution time went from 12sec prior to the fix, to less than second after the fix

haneslinger commented 7 months ago

sick, everything good now. @gtfierro @TShapinsky can I have a review?