Skip to content
Snippets Groups Projects

Use shared pointers for attribute systems.

Merged Use shared pointers for attribute systems.
dcthomp/smtk:shared-attrib-sys into master
2 unresolved threads
Merged David Thompson requested to merge dcthomp/smtk:shared-attrib-sys into master
2 unresolved threads

This change makes common::Resource and attribute::System shared pointer classes.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • David Thompson
    David Thompson @dcthomp ·
    Author Maintainer

    Do: test

  • Kitware Robot
    Kitware Robot @kwrobot ·
    Owner

    Errors:

    • commit b66c877b is not allowed because the following files are not formatted according to the 'autopep8' check: smtk/attribute/testing/python/basicAttributeDerivationTest.py, smtk/attribute/testing/python/definitionDefaultValueTest.py, smtk/io/testing/python/attributeReaderTest.py, smtk/model/testing/python/modelAttributes.py. Use Do: reformat to rewrite the MR source branch automatically.
    • commit b66c877b is not allowed because the following files are not formatted according to the 'clang-format' check: smtk/io/AttributeReader.h, smtk/io/AttributeWriter.h, smtk/io/SaveJSON.cxx, smtk/io/testing/cxx/ResourceSetTest.cxx, smtk/io/testing/cxx/ResourceSetWriterTest.cxx, and 2 others. Use Do: reformat to rewrite the MR source branch automatically.

    Please rewrite commits to fix the errors listed above (adding fixup commits will not resolve the errors) and force-push the branch again to update the merge request.

  • buildbot
    buildbot @buildbot ·
    Maintainer

    This merge request has been queued for testing. Test results may be viewed on CDash or Buildbot (only visible from inside Kitware).

    Branch-at: b66c877b

  • David Thompson mentioned in merge request cmb!378 (merged)

    mentioned in merge request cmb!378 (merged)

  • David Thompson added 5 commits

    added 5 commits

    Compare with previous version

  • David Thompson
    David Thompson @dcthomp ·
    Author Maintainer

    Do: test

  • Kitware Robot
    Kitware Robot @kwrobot ·
    Owner

    Errors:

    • commit 71926e35 is not allowed because the following files are not formatted according to the 'autopep8' check: smtk/attribute/testing/python/basicAttributeDerivationTest.py, smtk/attribute/testing/python/definitionDefaultValueTest.py, smtk/io/testing/python/attributeReaderTest.py, smtk/model/testing/python/modelAttributes.py. Use Do: reformat to rewrite the MR source branch automatically.
    • commit 71926e35 is not allowed because the following files are not formatted according to the 'clang-format' check: smtk/common/Resource.h, smtk/io/AttributeReader.h, smtk/io/AttributeWriter.h, smtk/io/SaveJSON.cxx, smtk/io/testing/cxx/ResourceSetTest.cxx, and 3 others. Use Do: reformat to rewrite the MR source branch automatically.

    Please rewrite commits to fix the errors listed above (adding fixup commits will not resolve the errors) and force-push the branch again to update the merge request.

  • buildbot
    buildbot @buildbot ·
    Maintainer

    This merge request has been queued for testing. Test results may be viewed on CDash or Buildbot (only visible from inside Kitware).

    Branch-at: 71926e35

  • David Thompson added 1 commit

    added 1 commit

    • 12366fed - Use shared pointers for attribute systems.

    Compare with previous version

  • Kitware Robot
    Kitware Robot @kwrobot ·
    Owner

    Errors:

    • commit 12366fed is not allowed because the following files are not formatted according to the 'autopep8' check: smtk/attribute/testing/python/basicAttributeDerivationTest.py, smtk/attribute/testing/python/definitionDefaultValueTest.py, smtk/io/testing/python/attributeReaderTest.py, smtk/model/testing/python/modelAttributes.py. Use Do: reformat to rewrite the MR source branch automatically.
    • commit 12366fed is not allowed because the following files are not formatted according to the 'clang-format' check: smtk/common/Resource.h, smtk/io/AttributeReader.h, smtk/io/AttributeWriter.h, smtk/io/SaveJSON.cxx, smtk/io/testing/cxx/ResourceSetTest.cxx, and 3 others. Use Do: reformat to rewrite the MR source branch automatically.

    Please rewrite commits to fix the errors listed above (adding fixup commits will not resolve the errors) and force-push the branch again to update the merge request.

  • David Thompson
    David Thompson @dcthomp started a thread on the diff
smtk/attribute/testing/python/copyAttributeTest.py
134 137 if smtk.wrappingProtocol() == 'pybind11':
135 test_system.setRefModelManager(model_manager)
136 options = smtk.attribute.System.CopyOptions.COPY_ASSOCIATIONS
137 test_system.copyAttribute(second_concrete, True)
138 test_system.copyAttribute(second_concrete, True, int(options))
138 139 else:
139 test_system.setRefModelManager(model_manager)
140
141 # Copy SecondConcrete attribute
142 # Note: shiboken refuses to wrap smtk::attribute::System::CopyOptions enum, saying:
143 # enum 'smtk::model::Manager::CopyOptions' is specified in typesystem, but not declared
144 # Rather than trying to reason with shiboken, the options are specified
145 # numerically
146 options = 0x00000001 # should be smtk.attribute.System.CopyOptions.COPY_ASSOCIATIONS
147 test_system.copyAttribute(second_concrete, options)
140 test_system.copyAttribute(second_concrete, True, options)
  • David Thompson
    David Thompson @dcthomp ·
    Author Maintainer

    @tjcorona @john.tourtellott Could you please take a look at the changes to the copyAttributeTest? I think the original was broken and there may still be some enum bit-values out of sync. The issues are:

    1. The options value was being ignored by both shiboken and pybind.
    2. Passing in smtk.attribute.System.CopyOptions.COPY_ASSOCIATIONS fails because the options are passed unmodified to smtk::attribute::Item::assign() which expects a value from smtk.attribute.Item.AssignmentOptions (which has COPY_MODEL_ASSOCIATIONS).

    I think I've fixed the test, but perhaps the enum values in System and Item need to be harmonized.

  • Please register or sign in to reply
  • T.J. Corona
    T.J. Corona @tjcorona ·
    Guest

    Sure thing! I'd be happy to review and to help with the pybind issue on Monday.

  • David Thompson
    David Thompson @dcthomp ·
    Author Maintainer

    Do: test -i praxis

  • buildbot
    buildbot @buildbot ·
    Maintainer

    This merge request has been queued for testing. Test results may be viewed on CDash or Buildbot (only visible from inside Kitware).

    Branch-at: 12366fed

  • David Thompson
    David Thompson @dcthomp ·
    Author Maintainer

    Do: reformat

  • Kitware Robot
    Kitware Robot @kwrobot ·
    Owner

    This topic has been reformatted and pushed; please fetch from the repository before further development to use the new topic locally.

  • Kitware Robot added 1 commit

    added 1 commit

    • fd5d2e08 - Use shared pointers for attribute systems.

    Compare with previous version

  • Haocheng LIU
    Haocheng LIU @haocheng.liu started a thread on the diff
    • smtk/attribute/testing/cxx/attributeAssociationTest.cxx
    28 28 {
    29 29 // ----
    30 30 // I. First see how things work when System is not yet set.
    31 attribute::System sys;
    31 attribute::SystemPtr sysptr = attribute::System::create();
    32 attribute::System& sys(*sysptr.get());
  • Haocheng LIU
    Haocheng LIU @haocheng.liu ·
    Contributor

    Looks decent to me.

    +1 except for attribute tests. I do not know about them enough to review.

  • David Thompson
    David Thompson @dcthomp ·
    Author Maintainer

    Do: merge

  • David Thompson mentioned in commit 2ceeea71

    mentioned in commit 2ceeea71

  • Kitware Robot merged

    merged

    • Please register or sign in to reply