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

Can we reuse this library for other BIM apps? #3

Open
Moult opened this issue Dec 7, 2019 · 17 comments
Open

Can we reuse this library for other BIM apps? #3

Moult opened this issue Dec 7, 2019 · 17 comments

Comments

@Moult
Copy link

Moult commented Dec 7, 2019

Would you be interested in isolating the portion of code that does reading / writing of a BCF file into a data structure into its own repo? That would allow other open-source programs who want to add BCF support to use it too easily :) I'd like to add BCF support into Blender :)

@podestplatz
Copy link
Owner

Hey Dion!
That is a great idea! 😃
I didn't work with the code in some time now, but as far as I can remember it shouldn't be too hard. As I am currently quite busy, I don't know how fast I can get to it.

Just to clarify: do you think about just the code that does the reading and writing, or would you also benefit from the datastructure itself when implementing BCF support in Blender? Because the reader and writer module (as I called them) heavily depend on the data structures.

Cheers,
Patrick

@Moult
Copy link
Author

Moult commented Dec 9, 2019

Hey @podestplatz - the data structures are fine, so long as they don't have anything to do with FreeCAD :) Then Blender can reuse those data structures! If you make a start, perhaps I can also help chip in - but you'd be the best as you're the most familiar with writing it!

By the way, I plan to integrate BCF support into the existing OpenBIM / IFC authoring tools I'm working on for Blender (https://blenderbim.org) - based of Thomas' original IFC importer he wrote for Blender. I have also written an IFC clash detector, which I can integrate with this BCF library!

@podestplatz
Copy link
Owner

Perfect! The datastructures are separate from FreeCAD so we are good to go.
I don't know when, but as soon as I get to it I will inform you 😉
Thanks for your willingness to help :)

That sounds great! I would like to assist you in your plans, if I am able to with my limited blender knowledge :D (apart from the separated BCF reading/writing repository)

@Moult
Copy link
Author

Moult commented Jan 6, 2020

1 month courtesy bump :)

@podestplatz
Copy link
Owner

I know I should have come back to you over the last days or so. I am sorry, but I did not forget.
For Saturday I have scheduled in some time to look into it a bit, by then you will have heard from me!

@podestplatz
Copy link
Owner

I looked into it a bit now. The data structure plus the reading and writing to BCF files is already nicely separated into its own module. However the test cases for the reader don't run through, so will have to look into that! As long as not all of the existing test cases succeed I will work locally. On Tuesday I will do the next session.

Cheers!

@Moult
Copy link
Author

Moult commented Mar 20, 2020

I'm revisiting this now, with a few questions :)

  • doing import bcfplugin leads to a lot of import issues when I'm trying to use it headlessly. In particular, pyperclip, pivy, FreeCAD and FreeCADGui, and PySide should be isolated so that people don't need to load it unless they want GUI behaviour. I'll submit a PR where I will attempt to clean this up a little, and would really appreciate your comments.
  • In files like topic.py, why are the class attribute names different to that in the spec? e.g. assignee maps to AssignedTo. Why not just keep them the same?
  • Again, in files like topic.py, is it possible to just read the spec's xsd file and then provide the attributes dynamically created from that? Is there really a need to hard-code them?

@Moult
Copy link
Author

Moult commented Mar 20, 2020

I have created a fork here:
https://github.com/Moult/BCF-Plugin-FreeCAD/tree/fix-headless

I aggressively removed any code that was FreeCAD or GUI only in an attempt to create a very lean minimal headless library. This change of course completely breaks its usefulness as a FreeCAD plugin :) But hopefully re-inserting the FreeCAD specific code as a wrapper on top is not too hard.

With your permission, perhaps this stripped down version could be turned into a python module simply named bcf, and then the FreeCAD plugin can be bcfplugin and import bcf?

Would it also be possible to clarify how you run the test suite? I tried but kept on getting a lot of failed tests... perhaps I am doing something fundamentally wrong.

@Moult
Copy link
Author

Moult commented Mar 23, 2020

@podestplatz I am proposing to buildingSMART that this implementation of BCF be added to their list of "reference" implementations for Python :) To do so, it will need to pass a series of test cases. I am currently working through their test case library and will keep you posted!

@podestplatz
Copy link
Owner

Hi @Moult,
I am taken aback by your investment in my little project from last summer. Thank you very much for the contributions you make. Unfortunately, in recent days I don't really find time to develop further on this project, which you might have guessed, thus I am even more grateful that you are proposing this implementation as reference to buildingSMART.

However, since we are requested by our government to stay at home due to COVID-19, I think I will find the time on Saturday to think about the points you raised in comment 1 and comment 2.

Cheers!

@Moult
Copy link
Author

Moult commented Mar 24, 2020

Just stating for the record that in my fix-headless fork, I have:

  • Added 2.0 as a supported version, as I was running BCF 2.0 test cases too, but I might drop this in the future. See here.
  • Fixed broken mod author deep copy here

Feel free to cherry pick the git commits if you'd like to maintain this FreeCAD plugin repo :) Alternatively, if you do decide to rewrite this plugin as a wrapper on top of the headless repo, that also works (and IMHO works better - as then the headless repo can be used for Blender, and buildingSMART too!)

Stay safe with COVID-19 :) All the best!

@podestplatz
Copy link
Owner

Hi @Moult,
I now have cloned your repo, to checkout the changes you made and to try to make the test cases/suit more understandable. But first I will answer some of your questions:

In files like topic.py, why are the class attribute names different to that in the spec? e.g. assignee maps to AssignedTo. Why not just keep them the same?

Well, back then it just seemed more sensible to call these properties that way. In fact I didn't like some of the official names. In hindsight, it would probably have been better to just use the official names, or create a file mapping official name to my name.


doing import bcfplugin leads to a lot of import issues when I'm trying to use it headlessly. In particular, pyperclip, pivy, FreeCAD and FreeCADGui, and PySide should be isolated so that people don't need to load it unless they want GUI behaviour. I'll submit a PR where I will attempt to clean this up a little, and would really appreciate your comments.

and

With your permission, perhaps this stripped down version could be turned into a python module simply named bcf, and then the FreeCAD plugin can be bcfplugin and import bcf?

As you already know, I appreciate you doing this really much! Since your idea, of separating the bcf library from the FreeCAD specific stuff and moving this into a wrapper, is awesome, I am planning of opening a feature branch feature/true-headlessness where I first incorporate your pull-request, and then move on to creating the FreeCAD wrapper.


Again, in files like topic.py, is it possible to just read the spec's xsd file and then provide the attributes dynamically created from that? Is there really a need to hard-code them?

In fact, in the early days, I planned to read the xsd files for their constraints they put upon some xml files. However, since this soon got quite complicated I ditched it. I never really thought about parsing the xsd file for the attributes themselves.


Would it also be possible to clarify how you run the test suite? I tried but kept on getting a lot of failed tests... perhaps I am doing something fundamentally wrong.

Unfortunately, my "test suite" grew with the project (as it should) but in later stages I verified the code only with the new tests I devised, and didn't check the ones I had already created. This probably led to the behavior we experience right now. I will see to it that I investigate the causes of the failing tests (that concern your proposed bcf module) and push them, if you permit, to your fix-headless branch, so we can merge the changes nicely into feature/true-headlessness, however, if you have a better idea on how to proceed in that matter (I don't know much about the best practice workflow) please share it with me :)

Until then, have a nice weekend and you too stay healthy in our current global situation.
Cheers!

@podestplatz
Copy link
Owner

I have created a fork here:
https://github.com/Moult/BCF-Plugin-FreeCAD/tree/fix-headless

I aggressively removed any code that was FreeCAD or GUI only in an attempt to create a very lean minimal headless library. This change of course completely breaks its usefulness as a FreeCAD plugin :) But hopefully re-inserting the FreeCAD specific code as a wrapper on top is not too hard.

With your permission, perhaps this stripped down version could be turned into a python module simply named bcf, and then the FreeCAD plugin can be bcfplugin and import bcf?

Would it also be possible to clarify how you run the test suite? I tried but kept on getting a lot of failed tests... perhaps I am doing something fundamentally wrong.

I have changed some things in the "test suite" regarding the reader-tests.py and writer-tests.py based on your branch fix-headless. Attached you will find the patch file (as txt file, because github wouldn't let me otherwise), I actually wanted to create a pull request, but I didn't want to fork your fork of my repo ^^
0001-Fix-reader-tests.py-and-some-errors-in-writer-tests.patch.txt

@Moult
Copy link
Author

Moult commented Mar 30, 2020

I have applied your patch (and fixed a viewpoint ordering bug too) in the fork: https://github.com/Moult/BCF-Plugin-FreeCAD/commits/fix-headless

Can I recommend either one of us creating a new repo, e.g. github.com/podestplatz/bcf or github.com/Moult/bcf, and then we can push the fix-headless code to that repo, and that that becomes the "pure" BCF library. Both of us can have read/write access to the repo - no need for forks, patchfiles, and extra branching. Then this repo becomes purely the FreeCAD repo? Then the two things are nicely separated. You can continue converting the FreeCAD plugin into a wrapper in your true-headlessness branch, and when it is working merge into master :)

@podestplatz
Copy link
Owner

Yeah sure, that is, well, a great idea! I will create one by, at the latest, the end of the week and invite you to it ;)

@Moult
Copy link
Author

Moult commented Apr 12, 2020

Just a heads up that I have temporarily held off pushing it to the repo as there are currently discussions ongoing about the possibility of buildingSMART publicly endorsing it, and therefore having a buildingSMART/bcf-python repo.

@Moult
Copy link
Author

Moult commented Apr 20, 2020

I've pushed to the repo and removed parts of the readme related to FreeCAD :)

The buildingSMART BCF group has decided against created an official buildingSMART repo (more work for them) and instead add it to their implementations page ... see here for my request: https://forums.buildingsmart.org/t/standards-implementation-database-updated/2222/6

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

No branches or pull requests

2 participants