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.

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

Update 223P Templates #225

Merged
merged 27 commits into from
Mar 10, 2023
Merged

Update 223P Templates #225

merged 27 commits into from
Mar 10, 2023

Conversation

gtfierro
Copy link
Collaborator

@gtfierro gtfierro commented Feb 25, 2023

I've made a note of future changes to 223P to support G36 concepts:

  • s223:HeatExchanger-Cooling/Heating not defined? (what are the right roles?)
  • s223:Role-HeatExchanger not defined but mentioned under s223:HeatExchanger
  • add Flow Status from G36?
  • inverse relations dont' seem to be added (connectsThrough -> connectsAt)
  • how is s223:MeasuredPropertyRule supposed to work? It seems to fire unnecessarily and add blank node properties everywhere -- does it need a condition?
  • missing FCU in 223P

I've added temporary fixes to 223P.ttl here so that our templates are valid. I recommend we merge this in w/o the changes pushed to 223P so that we're not dependent upon an external timeline.

@gtfierro gtfierro changed the base branch from main to develop February 25, 2023 06:25
@gtfierro gtfierro marked this pull request as ready for review February 25, 2023 20:27
@codecov-commenter
Copy link

codecov-commenter commented Feb 25, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (12ebb4e) 72.75% compared to head (d188c17) 72.75%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #225   +/-   ##
========================================
  Coverage    72.75%   72.75%           
========================================
  Files           32       32           
  Lines         2063     2063           
========================================
  Hits          1501     1501           
  Misses         562      562           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@TShapinsky TShapinsky left a comment

Choose a reason for hiding this comment

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

I think this is looking good. See my comment

@gtfierro gtfierro force-pushed the gtf-update-223p-templates branch from 2ca836e to eed1a3d Compare March 2, 2023 16:05
@gtfierro
Copy link
Collaborator Author

gtfierro commented Mar 3, 2023

@MatthewSteen I made a change to the cd.yml script just so this would pass the tests (it was using the old sphinx build instead of just jupyter book build). I was going to merge this in today but if you are planning to merge #224 soonish then I can hold off to make sure this works with the new build scripts

@MatthewSteen
Copy link
Member

@gtfierro the new docs uses sphinx (via jupyter book) for versions docs so make sure you're using whatever is on develop. I'm flying so will check later.

@gtfierro gtfierro merged commit 153bca2 into develop Mar 10, 2023
@gtfierro gtfierro deleted the gtf-update-223p-templates branch March 10, 2023 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants