-
Notifications
You must be signed in to change notification settings - Fork 194
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
Construct: the order of Computed
fields matter
#1106
Comments
No, the real cause of the problem is that Kaitai Struct This problem has been already discussed in #377 - apparently, @arekbulski added Nowadays, I hope it has been implemented since then, because https://construct.readthedocs.io/en/latest/lazy.html claims "This feature is fully implemented". |
Folks, I can also point to the elephant in the room — do we have capacity/interest to even support Construct target? Looks like @arekbulski unfortunately paused his participation in the project, so maybe we should follow the suit and disable/remove Construct support? |
In what project, Construct or KS? He seems to be still somewhat active in Construct: construct/construct#1080 (comment) I assume Construct is not dead, i.e. it is probably still used by people. https://pypistats.org/packages/construct can give some very rough idea (although you never know how much of that is bot traffic). The development of Construct might be stopped, but according to #377 (comment), that's supposedly because the project is "finished", which isn't inherently bad.
I don't see any good reason to do that. In my view, "supporting" the Construct target doesn't cost us anything we don't want. We can keep it as it is now and some people may still find it useful. And if someone wants, they can improve it. Not that long ago, there were some some PRs by @Mimickal (https://github.com/kaitai-io/kaitai_struct_compiler/pulls?q=is%3Apr+is%3Aopen+construct+sort%3Aupdated-desc), which we haven't gotten to yet, but they show interest, and also @wbarnha seemed interested at some point (kaitai-io/kaitai_struct_compiler#242 (comment)). Also kaitai-io/kaitai_struct_compiler#256 by @MatrixEditor (which I assume was closed for the lack of our response). (Speaking of hot candidates for removal, I think |
For some context, I was contributing to Construct support because at the time it was the only way to achieve serialization with Kaitai Struct. I've been away from the project that needed that for some time, but I've been keeping an eye on the space. As far as I can tell, there still isn't a great solution for data-driven struct generation like KSC anywhere. Aside from the dev time needed to support it, I don't think there's a good case for removing Construct support from Kaitai until Kaitai's own Python generation supports serialization. Then at least there would be an alternative. |
Currently some tests on https://ci.kaitai.io/ are failed because of that fact that instance fields are sorted alphabetically (due to SortedMap used for storage) and generated in the same order.
Sorting of instances are good thing which will ensure stable output friendly for diffs, but Construct currently cannot calculate dependents inside of
Computed
fields and raises an error instead.Although problem seems to be resolved on Construct side, it also could be useful to generate
Computed
properties in a topological order of dependencies.The text was updated successfully, but these errors were encountered: