RosettaCommons / binder

Binder, tool for automatic generation of Python bindings
MIT License
322 stars 66 forks source link

Add option to skip binding a field #257

Closed jlblancoc closed 1 year ago

jlblancoc commented 1 year ago

This commit is based on a larger commit by @charbeljc in https://github.com/RosettaCommons/binder/pull/160 but manually cherry-picking just the "-field" feature.

jlblancoc commented 1 year ago

Sure! One doubt: what version of clang-format are you using to format the files? I tried clang-format-11 and clang-format-15 and both changes much more than just my changes...

lyskov commented 1 year ago

@jlblancoc re clang-format version: unless i am mistaken it was v13

jlblancoc commented 1 year ago

Hi @lyskov ! It's done now:

There are other weird things I noticed in the way, but kept changes minimal in this PR anyway, please check them for subsequent commits?:

That said... congrats for the huge project, it literally made my life easier :-)

lyskov commented 1 year ago

Thank you for your feedback and for adding test for this @jlblancoc !

  • test/self-test.py only works with python2.7. I had to manually change a couple of lines to make it work with python3.

-- yes, thank you for brining this up! I am aware of this and will escalate update to Python-3.

  • no clang-format version seems to be actually applied to existing sources (some of them).

-- thank you for letting me know! Looks like quite a few changes got accumulated since last clang format run, - i will looks this up.

  • It's confusing why only part of the test/*.hpp files are handled at the cmake + make test level, but then there is an apparently independent way to run tests via "build-and-run-tests.py" (??).

-- cmake tests does not compare test results (only makes sure that results is compilable) and they also require system-wide install of LLVM which is not always practical during development.

Again, thank you for your PR and feedback @jlblancoc ! I will merge this shortly.

jlblancoc commented 1 year ago

Got it. Thanks you!