Closed Moult closed 2 years ago
Proposal. Create three categories:
Content. Fix immediately, before end of Feb
Appearance. I agree, I think the mini.css framework came from you, but the tables and whitespace are also not to my liking. Fix before end of Mar. Proposals?
Code quality. Well, mixing HTML and Py is a given considering the fact that we do HTMl to markdown conversion, inline SVG processing and what not. tabulate()
is a really quick hack to get table content, if somebody wants to rework that into jinja templates I'm fine, but I'm personally not convinced it we will gain a lot from it.
What does need to happen before Mar though is that we have more understanding of the overall contents of the overall site instead of blindly catting the contents of Markdown to HTML. We need consistent figure/table numbering, preferably also with a listing of figures. Proper captions. If that means we need some sort of internal intermediate representation of the overall content, I'm quite sure we can also leverage that to move the markup outside of the py code.
Docker is not necessary for development (you'd only miss the search). Just run create_resources.sh
which parses the XMI into a bunch of json artefacts and then FLASK_APP=server.py FLASK_ENV=development flask run
Edit: missing images is also content obviously, but should be easily resolved?
Ok, my meeting ended quickly, going over some of the points:
Psets show "edit in github" but it doesn't make sense
It does. The pset have a definition you can edit. The individual props have the little gray button inline in the table. It's probably a content problem that not all definitions for new psets have made it into the markdown yet.
Pset pages don't show what it is applicable to
Yes these kind of oversights there are quite a bit still I think. Other example, documentation for where rules doesn't make it to the HTML. Not even what's the status of derived and unique clauses. Just iterate quickly, these are really fast to address, but maybe it's time to build a public list.
Changelogs
Agreed, they are also not to my liking yet in terms of content
Notes in chapter 3 terms should not run inline
I think we should transform it into a table (or dd). Along the lines of what we do with entity attribute tables.
Does search not do partial matches of page titles? E.g. searching IfcAir won't come up with IfcAirTerminal
Feel free to look this up in solr. I'm very unfamiliar with it. I'd also really like to do some sort of advanced search so that you can limit the sections in which it searches.
Good idea - recategorised. Agreed that code quality is a low priority right now. Maybe there is no value in it.
I get this error, which package is this from?
* Serving Flask app "server.py" (lazy loading)
* Environment: development
* Debug mode: on
* Running on http://127.0.0.1:5000/ (Press CTRL+C to quit)
* Restarting with stat
* Debugger is active!
* Debugger PIN: 155-459-101
127.0.0.1 - - [10/Feb/2022 19:30:33] "GET / HTTP/1.1" 500 -
Traceback (most recent call last):
File "/home/dion/Projects/IFC4.3.x-development/code/server.py", line 30, in <module>
import md as mdp
File "/home/dion/Projects/IFC4.3.x-development/code/md.py", line 12, in <module>
from markdown_it.utils import AttrDict
ImportError: cannot import name 'AttrDict' from 'markdown_it.utils' (/home/dion/Projects/env/lib/python3.9/site-packages/markdown_it/utils.py)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/dion/Projects/env/lib/python3.9/site-packages/flask/cli.py", line 338, in __call__
self._flush_bg_loading_exception()
File "/home/dion/Projects/env/lib/python3.9/site-packages/flask/cli.py", line 326, in _flush_bg_loading_exception
reraise(*exc_info)
File "/home/dion/Projects/env/lib/python3.9/site-packages/flask/_compat.py", line 39, in reraise
raise value
File "/home/dion/Projects/env/lib/python3.9/site-packages/flask/cli.py", line 314, in _load_app
self._load_unlocked()
File "/home/dion/Projects/env/lib/python3.9/site-packages/flask/cli.py", line 330, in _load_unlocked
self._app = rv = self.loader()
File "/home/dion/Projects/env/lib/python3.9/site-packages/flask/cli.py", line 388, in load_app
app = locate_app(self, import_name, name)
File "/home/dion/Projects/env/lib/python3.9/site-packages/flask/cli.py", line 245, in locate_app
raise NoAppException(
flask.cli.NoAppException: While importing "server", an ImportError was raised:
Traceback (most recent call last):
File "/home/dion/Projects/env/lib/python3.9/site-packages/flask/cli.py", line 240, in locate_app
__import__(module_name)
File "/home/dion/Projects/IFC4.3.x-development/code/server.py", line 30, in <module>
import md as mdp
File "/home/dion/Projects/IFC4.3.x-development/code/md.py", line 12, in <module>
from markdown_it.utils import AttrDict
ImportError: cannot import name 'AttrDict' from 'markdown_it.utils' (/home/dion/Projects/env/lib/python3.9/site-packages/markdown_it/utils.py)
Have a look https://github.com/buildingSMART/IFC4.3.x-development/blob/master/code/Dockerfile#L10 some versions are pinned.
Edit: nevermind, will submit PR with gitignored svgs dir :)
The markdown files are the source of truth for docs now, right? So the reason the images aren't showing up are because the images are actually borked here: https://github.com/buildingSMART/IFC4.3.x-development/blob/master/docs/schemas/shared/IfcSharedBldgElements/Entities/IfcWall.md?plain=1#L83
I can fix these in bulk, but I just wanted to confirm with you before I start bulk editing these markdown files that they aren't being generated from somewhere else.
It seems the error is here https://github.com/buildingSMART/IFC4.3.x-development/blob/master/code/import_concepts_to_markdown.py#L68-L69 ðŸ˜
Is it fixed, just if we add the trailing slash? Then I prefer to fix there and rerun the import as the bsi-bot user.
Well, or actually, maybe sed
is still better, but also fix in the code. Still propose to commit as bsi-bot
No, that is part of the problem, but probably not all since it's not just the trailing slash, it seems as though some string cleaning occurred (underscores stripped) which was probably a bit heavy handed.
I'm not following. The case you referenced is
Where
![curved wall axis](../../../../figuresifcwallstandard_curvedwall_01-layout1.gif)
should be
![curved wall axis](../../../../figures/ifcwallstandard_curvedwall_01-layout1.gif)
What else did you observe?
Oh, this is what I see in HTML - underscores are gone:
<p><img alt="curved wall axis" src="../figuresifcwallstandardcurvedwall01-layout1.gif"/></p>
Maybe a Markdown thing? Didn't investigate closely yet.
Hm, that's here then https://github.com/buildingSMART/IFC4.3.x-development/blob/master/code/server.py#L582 Not entirely sure why we did that. Can probably be removed.
Cool, since this is a bsi-bot thing maybe you should fix this one :)
Also is there a missing Pset_Address xml somewhere?
Not XML but probably the Md has nog been created yet.
I've "fixed" changelogs, not that it's a super high priority, but because it allows me to demonstrate how I think the templating should work. Still a lot to work and I only barely touched the visuals, but the idea is that the servers job is purely to derive semantics from the markdown, then the markup can truly do its job as markup like this: https://github.com/buildingSMART/IFC4.3.x-development/blob/master/code/templates/entity.html#L23-L56 - I'd expect the same for every single other standardised content section.
I didn't get a chance to do much code today since I was busy triaging bugs (finished the entirety of 2020!) but this weekend I plan to do a lot more hacking on this.
Thanks. For me the more urgent need in terms of refactoring is getting the code modular on both sides, so that we have e.g change logs on psets as well.
Like I said. I think html is a pretty good data model that allows us to do the figure numbering by section as a simple tree traversal. Building that in the template would be too late to derive a figure listing from it. Let's keep an eye on that.
Sent from a mobile device. Excuse my brevity. Kind regards, Thomas
Op vr 11 feb. 2022 08:49 schreef Dion Moult @.***>:
I've "fixed" changelogs, not that it's a super high priority, but because it allows me to demonstrate how I think the templating should work. Still a lot to work and I only barely touched the visuals, but the idea is that the servers job is purely to derive semantics from the markdown, then the markup can truly do its job as markup like this: https://github.com/buildingSMART/IFC4.3.x-development/blob/master/code/templates/entity.html#L23-L56
- I'd expect the same for every single other standardised content section.
I didn't get a chance to do much code today since I was busy triaging bugs (finished the entirety of 2020!) but this weekend I plan to do a lot more hacking on this.
— Reply to this email directly, view it on GitHub https://github.com/buildingSMART/IFC4.3.x-development/issues/179#issuecomment-1035953306, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAILWV2DX2RPQXDMLHAJRVTU2S5QHANCNFSM5N7WGFIA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you commented.Message ID: @.***>
Yes agreed :) I was going to limit template placing to bespoke structured sections like changelog, attribute table, inheritance tree.
Sent from ProtonMail mobile
-------- Original Message -------- On 11 Feb 2022, 7:12 pm, Thomas Krijnen wrote:
Thanks. For me the more urgent need in terms of refactoring is getting the code modular on both sides, so that we have e.g change logs on psets as well.
Like I said. I think html is a pretty good data model that allows us to do the figure numbering by section as a simple tree traversal. Building that in the template would be too late to derive a figure listing from it. Let's keep an eye on that.
Sent from a mobile device. Excuse my brevity. Kind regards, Thomas
Op vr 11 feb. 2022 08:49 schreef Dion Moult @.***>:
I've "fixed" changelogs, not that it's a super high priority, but because it allows me to demonstrate how I think the templating should work. Still a lot to work and I only barely touched the visuals, but the idea is that the servers job is purely to derive semantics from the markdown, then the markup can truly do its job as markup like this: https://github.com/buildingSMART/IFC4.3.x-development/blob/master/code/templates/entity.html#L23-L56
- I'd expect the same for every single other standardised content section.
I didn't get a chance to do much code today since I was busy triaging bugs (finished the entirety of 2020!) but this weekend I plan to do a lot more hacking on this.
— Reply to this email directly, view it on GitHub https://github.com/buildingSMART/IFC4.3.x-development/issues/179#issuecomment-1035953306, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAILWV2DX2RPQXDMLHAJRVTU2S5QHANCNFSM5N7WGFIA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you commented.Message ID: @.***>
— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you authored the thread.Message ID: @.***>
I noticed there is no data extracted for whether an entity is abstract. I can put this in a new abstract_entities.json . Is that OK? The old docs showed abstractness in the tree which I want to build.
Yes, abstract and deprecated should still be shown in the tree. I think a new json is fine, but we after Feb we should also reserve some time to unify this into a comprehensive data structure probly.
Cheers, it did feel hackish, I'll try my best to minimise hacks.
Enough hacking for today, hopefully I didn't break anything too badly so far. Sorry about the cover photo :(
I like it!
Maybe the abstract non-abstract difference could be a little bit more pronounced?
I'm a bit in doubt with the left-aligned diagram. Benefit is that it's easy to find the focussed entity. But letting graphviz more in charge of the layout with more neutral weights (maybe remove the forced node width) might result in the ability to put more nodes into the diagram. Did you experiment with that by any chance?
I disabled the width by accident and really didn't like it. The new left aligned thing is growing on me but I will play more tomorrow :) sleep time!
Sent from ProtonMail mobile
-------- Original Message -------- On 11 Feb 2022, 10:39 pm, Thomas Krijnen wrote:
I like it!
Maybe the abstract non-abstract difference could be a little bit more pronounced?
I'm a bit in doubt with the left-aligned diagram. Benefit is that it's easy to find the focussed entity. But letting graphviz more in charge of the layout with more neutral weights (maybe remove the forced node width) might result in the ability to put more nodes into the diagram. Did you experiment with that by any chance?
— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you authored the thread.Message ID: @.***>
I did some work on section number generation. Now all headers are collapsible, and a bunch of number oddities seem to be fixed. In addition now changelogs are numbered too. I've also split out concept usage (like the existing docs) which previously was mixed into inheritance. I'm also toying with the idea of generating a page table of contents like Wikipedia does (it might make more sense with the new layout). I'll also add section anchors.
I'd like to help tackling parsing and labeling figures, examples, notes, so that they can be rendered nicely instead of as you say just catting the markdown. What's the rule on numbering figures and tables? In the old IFC it seems to be just starting at 1 (from where?) then going up.
See before:
and after:
What's the rule on numbering figures and tables?
I don't think there is a rule, as long as the numbering is unique. When there is a mechanism in place it should be easy to adjust. For now I'd say <chapter>.<figure>
so that we can at least test the numbering takes into account info from the structure as well as does consistent numbering across multiple pages.
Re TOC. I think when we do some sort of traversal for the images, what will come out of it is that we also know all sections in the spec. So I think we can also use that to augment the TOC in the left side bar. Is that what you mean?
For TOC you know how Wikipedia has a summary of the headers with anchors you can jump to at the top of a long page? We could do the same - pull the latest code and you'll see I'm experimenting with a third column - could fit nicely there potentially. For long pages like IfcWall it could be useful.
Do you have a personal TODO list for the doc generation I can help out with or should I just continue randomly doing stuff here and there? I'd like to help offload the work so you can focus on schema modeling :)
In the docs there are these "HISTORY" notes. Can / should these be purged and instead just be reflected in the changelog? It seems odd to have them inline.
Your code is live now. I'm quite happy we're back to white bg. Great work overall. Edit: it's not white, but the overall layout is really improved.
I experienced a few glitches with the headings
click IFC4, click change log
Click concept usage
I think this is a bit much. Semantically I also wouldn't see this as a warning.
I had a go with this (mockup only)
In the docs there are these "HISTORY" notes. Can / should these be purged and instead just be reflected in the changelog? It seems odd to have them inline.
Yes, that's also an open issue still. We have computer generated change logs, but their intent is missing. Do you have ideas on how to represent them in the change log?
We could have a CHANGES.md or something:
# Changes
## IfcRoot
### IFC4
#### OwnerHistory
Made optional because we think it's cute.
Which is then augmented with the computer generated changelog into
That would be inline with a lot of the stuff we currently do.
Is derived from:
But the headings only serve as lookup keys into the table which is ultimately generated from XMI
Re toc. But shouldn't we blend it in with the left sidebar? If that one offers the complete spec hierarchy and we make sure it scrolls along with the reader it's basically the same? Maybe with the added advantage that you can cross reference to other entities.
Like, you're looking at the Material Layer concept on Wall, and want to quickly jump to what it looks like on Slab. I think that would be amazing. Maybe we kind of already have that now that we can jump from [Entity] -> [Concept] -> [Other entity] though. I just really hope we can facilitate the reader in really experiencing a good hyperlink experience and not just consume this linearly.
A couple of screenshots for the curious - selects and enums now have numbered headers and show tables of values. Also notes/histories/whatever are at least tagged distinctly. Also notice in the screenshot the deprecation "note" that doesn't follow the conventions that we can start catching and solving using this method.
I've written a bunch of docs in code/README.md
- let me know if I made any inaccuracies :)
looks great!
Attributes
heading for applicabilitydocs/templates/README.md
which I think provided this intro paragraph was deleted here https://github.com/buildingSMART/IFC4.3.x-development/commit/c6bf4854427427cfaad597323ed1e0dd61d73f36 back in December last year
How should this be fixed? Manually? Or some import script rerun?
Good catch, fixed attributes typo: https://github.com/buildingSMART/IFC4.3.x-development/commit/2870627ba5a91dd7cd4e8aa7e6cb5a070155868d
Ah you're right, I didn't even know there was code for that. Thanks! Reverted the commit for docs/templates/README.md
.
Fyi the markdown fig caption failed in a couple of cases quickly patched it 3ec7c672293d1c2d908d6a9ba6824e1e1a8b1127
Tomorrow I will prioritize detecting 500s before we deploy to prod.
I don't know if it is related to this, but whenever an ENUM has an 'underscore', the parsing seems to skip the ENUM term and the description. I was seeing many of them missing.
E.g. ### T
should have been
### T_BEAM
A beam that forms part of a slab construction and acts together with the slab which its carries. Such beams are often of T-shape (therefore the English name), but may have other shapes as well, e.g. an L-Shape or an Inverted-T-Shape.
Super helpful @stefkeB thanks. Should be corrected now https://github.com/buildingSMART/IFC4.3.x-development/blob/master/docs/schemas/shared/IfcSharedBldgElements/Types/IfcBeamTypeEnum.md#t_beam
There are still a few known issues with the figure parsing, I hope to get them resolved over the next few days.
Should every single table be numbered too? (like every table used for every concept? It can be pretty hectic)
Cool. Yes tables too if you ask me.
Sent from a mobile device. Excuse my brevity. Kind regards, Thomas
Op ma 14 feb. 2022 22:06 schreef Dion Moult @.***>:
There are still a few known issues with the figure parsing, I hope to get them resolved over the next few days.
Should every single table be numbered too?
— Reply to this email directly, view it on GitHub https://github.com/buildingSMART/IFC4.3.x-development/issues/179#issuecomment-1039558001, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAILWV4QJDE5VHTCERKQLVDU3FVEPANCNFSM5N7WGFIA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you commented.Message ID: @.***>
So I'm getting 403 responses from the Github API now for the changes widget. Perhaps we queried them one too many times?
Object { message: "API rate limit exceeded for 110.33.208.24. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", documentation_url: "https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting" } ​> documentation_url: "https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting" ​> message: "API rate limit exceeded for 110.33.208.24. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)"
Just a case by case I guess if someone visits it every day
Perhaps we queried them one too many times?
Well, you queried them one too many times ;)
It's based on your local ip since it uses Javascript. We can do it server side. Then we can use the authenticated account, but we still need to do caching.
Psets names on the entity page (e.g. IfcCovering) should link to the respective pset doc page
I think they do, can you explain?
Yes, they seem to. No idea what I was on about :D will remove that text
Is there any reason why we don't have an auto incrementing number in the template? I.e {% set paragraph = 1 %}
and auto-increment from there.
It seems unnecessary to couple the number generation in Python with the order of the placement in Jinja
It seems unnecessary to couple the number generation in Python with the order of the placement in Jinja
Agreed. At the time whilst I was refactoring it when some stuff had numbers hardcoded and some didn't it just turned out that way in the code shuffling but it is very much a WIP. I'll come up with a better solution - not a personal high priority though.
I'm against setting and doing incrementing in the template though, I strongly believe in a Mustache approach of templating - only plain vars, if bools, and loops, nothing else :)
I tried to use http://ifc43-docs.standards.buildingsmart.org for daily doc browsing but really struggled and often had to revert to looking at existing IFC4 docs. This issue is to list all the things I'd like to see resolved (and help write the code to fix it too!) so I don't forget.
Content
Aesthetics
Edit: all fixed
Code
Edit: all fixed
Just a running list. Keen to hack but I need some handholding to make it easier to work with docker :(