Browse Source

python: fix inconsistent naming of Trade enum sets.

They should be named after the plural EnumSet, not the C++ enum. That
was already done for the enums in the primitives library as well as all
shader flags, but not here. They should all also contain a NONE value
for an empty set.

Breaking change, sorry. To avoid similar mistakes in the future, this is
now documented in the API Conventions page.
next
Vladimír Vondruš 3 years ago
parent
commit
2a2fa675b7
  1. 8
      doc/python/magnum.trade.rst
  2. 21
      doc/python/pages/api-conventions.rst
  3. 2
      doc/python/pages/changelog.rst
  4. 30
      src/python/magnum/test/test_trade.py
  5. 10
      src/python/magnum/trade.cpp

8
doc/python/magnum.trade.rst

@ -128,10 +128,10 @@
.. py:property:: magnum.trade.MeshData.mutable_index_data .. py:property:: magnum.trade.MeshData.mutable_index_data
:raise AttributeError: If :ref:`index_data_flags` doesn't contain :raise AttributeError: If :ref:`index_data_flags` doesn't contain
:ref:`DataFlag.MUTABLE` :ref:`DataFlags.MUTABLE`
.. py:property:: magnum.trade.MeshData.mutable_vertex_data .. py:property:: magnum.trade.MeshData.mutable_vertex_data
:raise AttributeError: If :ref:`vertex_data_flags` doesn't contain :raise AttributeError: If :ref:`vertex_data_flags` doesn't contain
:ref:`DataFlag.MUTABLE` :ref:`DataFlags.MUTABLE`
.. py:property:: magnum.trade.MeshData.index_count .. py:property:: magnum.trade.MeshData.index_count
:raise AttributeError: If :ref:`is_indexed` is :py:`False` :raise AttributeError: If :ref:`is_indexed` is :py:`False`
.. py:property:: magnum.trade.MeshData.index_type .. py:property:: magnum.trade.MeshData.index_type
@ -142,7 +142,7 @@
:raise AttributeError: If :ref:`is_indexed` is :py:`False` :raise AttributeError: If :ref:`is_indexed` is :py:`False`
.. py:property:: magnum.trade.MeshData.mutable_indices .. py:property:: magnum.trade.MeshData.mutable_indices
:raise AttributeError: If :ref:`index_data_flags` doesn't contain :raise AttributeError: If :ref:`index_data_flags` doesn't contain
:ref:`DataFlag.MUTABLE` :ref:`DataFlags.MUTABLE`
.. py:function:: magnum.trade.MeshData.attribute_name .. py:function:: magnum.trade.MeshData.attribute_name
:raise IndexError: If :p:`id` is negative or not less than :raise IndexError: If :p:`id` is negative or not less than
:ref:`attribute_count()` :ref:`attribute_count()`
@ -188,7 +188,7 @@
:raise KeyError: If :p:`id` is negative or not less than :raise KeyError: If :p:`id` is negative or not less than
:ref:`attribute_count()` for :p:`name` :ref:`attribute_count()` for :p:`name`
:raise AttributeError: If :ref:`vertex_data_flags` doesn't contain :raise AttributeError: If :ref:`vertex_data_flags` doesn't contain
:ref:`DataFlag.MUTABLE` :ref:`DataFlags.MUTABLE`
.. py:enum:: magnum.trade.SceneField .. py:enum:: magnum.trade.SceneField

21
doc/python/pages/api-conventions.rst

@ -100,6 +100,9 @@ C++ Python
`Enums`_ `Enums`_
-------- --------
Custom construction helpers for enums are converted to functions directly on
the Python enum class.
.. class:: m-table .. class:: m-table
============================================== ============================ ============================================== ============================
@ -107,6 +110,24 @@ C++ Python
============================================== ============================ ============================================== ============================
:dox:`PixelFormat::RGB8Unorm` :ref:`PixelFormat.RGB8_UNORM` :dox:`PixelFormat::RGB8Unorm` :ref:`PixelFormat.RGB8_UNORM`
:dox:`MeshPrimitive::TriangleStrip` :ref:`MeshPrimitive.TRIANGLE_STRIP` :dox:`MeshPrimitive::TriangleStrip` :ref:`MeshPrimitive.TRIANGLE_STRIP`
:dox:`Trade::meshAttributeCustom()` :ref:`trade.MeshAttribute.CUSTOM() <trade.MeshAttribute>`
:dox:`Trade::isMeshAttributeCustom()` :ref:`trade.MeshAttribute.is_custom <trade.MeshAttribute>`
============================================== ============================
`Enum sets`_
------------
Compared to C++, there's just one enum type with a plural name, and it contains
both the values and binary operators. Additionally, there's an explicit
``NONE`` value for an empty set.
.. class:: m-table
============================================== ============================
C++ Python
============================================== ============================
:dox:`Trade::DataFlag::Mutable` :ref:`trade.DataFlags.MUTABLE`
:dox:`Trade::DataFlags{} <Trade::DataFlags>` :ref:`trade.DataFlags.NONE`
============================================== ============================ ============================================== ============================
`Constants`_ `Constants`_

2
doc/python/pages/changelog.rst

@ -115,7 +115,7 @@ Changelog
:ref:`trade.AbstractSceneConverter` :ref:`trade.AbstractSceneConverter`
- Exposed the whole interface of :ref:`trade.MeshData` including typed access - Exposed the whole interface of :ref:`trade.MeshData` including typed access
to index and attribute data, together with :ref:`VertexFormat`, to index and attribute data, together with :ref:`VertexFormat`,
:ref:`trade.DataFlag`, :ref:`trade.AbstractImporter.mesh_attribute_name()` :ref:`trade.DataFlags`, :ref:`trade.AbstractImporter.mesh_attribute_name()`
and :ref:`trade.AbstractImporter.mesh_attribute_for_name()` and :ref:`trade.AbstractImporter.mesh_attribute_for_name()`
- Exposed the whole interface of :ref:`trade.SceneData` including typed - Exposed the whole interface of :ref:`trade.SceneData` including typed
access to mapping and field data, together with access to mapping and field data, together with

30
src/python/magnum/test/test_trade.py

@ -152,8 +152,8 @@ class MeshData(unittest.TestCase):
mesh = importer.mesh(0) mesh = importer.mesh(0)
self.assertEqual(mesh.primitive, MeshPrimitive.TRIANGLES) self.assertEqual(mesh.primitive, MeshPrimitive.TRIANGLES)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(mesh.index_data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
self.assertEqual(mesh.vertex_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(mesh.vertex_data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
# Index properties # Index properties
self.assertTrue(mesh.is_indexed) self.assertTrue(mesh.is_indexed)
@ -377,7 +377,7 @@ class MeshData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0) mesh = importer.mesh(0)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(mesh.index_data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
index_data = mesh.index_data index_data = mesh.index_data
mutable_index_data = mesh.mutable_index_data mutable_index_data = mesh.mutable_index_data
@ -394,7 +394,7 @@ class MeshData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0) mesh = importer.mesh(0)
self.assertEqual(mesh.vertex_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(mesh.vertex_data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
vertex_data = mesh.vertex_data vertex_data = mesh.vertex_data
mutable_vertex_data = mesh.mutable_vertex_data mutable_vertex_data = mesh.mutable_vertex_data
@ -411,7 +411,7 @@ class MeshData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0) mesh = importer.mesh(0)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(mesh.index_data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
indices = mesh.indices indices = mesh.indices
mutable_indices = mesh.mutable_indices mutable_indices = mesh.mutable_indices
@ -426,7 +426,7 @@ class MeshData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0) mesh = importer.mesh(0)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(mesh.index_data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
position_id = mesh.attribute_id(trade.MeshAttribute.POSITION) position_id = mesh.attribute_id(trade.MeshAttribute.POSITION)
positions = mesh.attribute(position_id) positions = mesh.attribute(position_id)
@ -450,7 +450,7 @@ class MeshData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0) mesh = importer.mesh(0)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(mesh.index_data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
packed_attribute = importer.mesh_attribute_for_name("_CUSTOM_PACKED_ATTRIBUTE") packed_attribute = importer.mesh_attribute_for_name("_CUSTOM_PACKED_ATTRIBUTE")
self.assertEqual(mesh.attribute_format(packed_attribute), VertexFormat.VECTOR3UB) self.assertEqual(mesh.attribute_format(packed_attribute), VertexFormat.VECTOR3UB)
@ -467,7 +467,7 @@ class MeshData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.gltf'))
mesh = importer.mesh(0) mesh = importer.mesh(0)
self.assertEqual(mesh.index_data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(mesh.index_data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
self.assertEqual(mesh.attribute_format(trade.MeshAttribute.COLOR), VertexFormat.VECTOR3UB_NORMALIZED) self.assertEqual(mesh.attribute_format(trade.MeshAttribute.COLOR), VertexFormat.VECTOR3UB_NORMALIZED)
normalized = mesh.attribute(trade.MeshAttribute.COLOR) normalized = mesh.attribute(trade.MeshAttribute.COLOR)
@ -483,8 +483,8 @@ class MeshData(unittest.TestCase):
mesh = primitives.cube_solid() mesh = primitives.cube_solid()
# TODO split this once there's a mesh where only one or the other would # TODO split this once there's a mesh where only one or the other would
# be true (maybe with zero-copy loading of PLYs / STLs?) # be true (maybe with zero-copy loading of PLYs / STLs?)
self.assertEqual(mesh.index_data_flags, trade.DataFlag(0)) self.assertEqual(mesh.index_data_flags, trade.DataFlags.NONE)
self.assertEqual(mesh.vertex_data_flags, trade.DataFlag(0)) self.assertEqual(mesh.vertex_data_flags, trade.DataFlags.NONE)
with self.assertRaisesRegex(AttributeError, "mesh index data is not mutable"): with self.assertRaisesRegex(AttributeError, "mesh index data is not mutable"):
mesh.mutable_index_data mesh.mutable_index_data
@ -674,7 +674,7 @@ class SceneData(unittest.TestCase):
self.assertEqual(scene.field_name(2), trade.SceneField.TRANSFORMATION) self.assertEqual(scene.field_name(2), trade.SceneField.TRANSFORMATION)
self.assertEqual(scene.field_name(6), trade.SceneField.CUSTOM(1)) self.assertEqual(scene.field_name(6), trade.SceneField.CUSTOM(1))
# TODO some field flags in glTF please? # TODO some field flags in glTF please?
self.assertEqual(scene.field_flags(2), trade.SceneFieldFlag(0)) self.assertEqual(scene.field_flags(2), trade.SceneFieldFlags.NONE)
self.assertEqual(scene.field_type(2), trade.SceneFieldType.MATRIX4X4) self.assertEqual(scene.field_type(2), trade.SceneFieldType.MATRIX4X4)
self.assertEqual(scene.field_size(3), 3) self.assertEqual(scene.field_size(3), 3)
# TODO add some array extras once supported to have this non-zero for # TODO add some array extras once supported to have this non-zero for
@ -694,7 +694,7 @@ class SceneData(unittest.TestCase):
self.assertEqual(scene.field_object_offset(trade.SceneField.TRANSFORMATION, 3), 2) self.assertEqual(scene.field_object_offset(trade.SceneField.TRANSFORMATION, 3), 2)
self.assertEqual(scene.field_object_offset(trade.SceneField.TRANSFORMATION, 3, 1), 2) self.assertEqual(scene.field_object_offset(trade.SceneField.TRANSFORMATION, 3, 1), 2)
# TODO some field flags in glTF please? # TODO some field flags in glTF please?
self.assertEqual(scene.field_flags(trade.SceneField.PARENT), trade.SceneFieldFlag(0)) self.assertEqual(scene.field_flags(trade.SceneField.PARENT), trade.SceneFieldFlags.NONE)
self.assertEqual(scene.field_type(trade.SceneField.CUSTOM(1)), trade.SceneFieldType.STRING_OFFSET32) self.assertEqual(scene.field_type(trade.SceneField.CUSTOM(1)), trade.SceneFieldType.STRING_OFFSET32)
self.assertEqual(scene.field_size(trade.SceneField.CUSTOM(0)), 1) self.assertEqual(scene.field_size(trade.SceneField.CUSTOM(0)), 1)
# TODO add some array extras once supported to have this non-zero for # TODO add some array extras once supported to have this non-zero for
@ -818,7 +818,7 @@ class SceneData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'scene.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'scene.gltf'))
scene = importer.scene(0) scene = importer.scene(0)
self.assertEqual(scene.data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(scene.data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
translation_id = scene.field_id(trade.SceneField.TRANSLATION) translation_id = scene.field_id(trade.SceneField.TRANSLATION)
translations = scene.mapping(translation_id) translations = scene.mapping(translation_id)
@ -842,7 +842,7 @@ class SceneData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'scene.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'scene.gltf'))
scene = importer.scene(0) scene = importer.scene(0)
self.assertEqual(scene.data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(scene.data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
translation_id = scene.field_id(trade.SceneField.TRANSLATION) translation_id = scene.field_id(trade.SceneField.TRANSLATION)
translations = scene.field(translation_id) translations = scene.field(translation_id)
@ -866,7 +866,7 @@ class SceneData(unittest.TestCase):
importer.open_file(os.path.join(os.path.dirname(__file__), 'scene.gltf')) importer.open_file(os.path.join(os.path.dirname(__file__), 'scene.gltf'))
scene = importer.scene(0) scene = importer.scene(0)
self.assertEqual(scene.data_flags, trade.DataFlag.OWNED|trade.DataFlag.MUTABLE) self.assertEqual(scene.data_flags, trade.DataFlags.OWNED|trade.DataFlags.MUTABLE)
pointer = scene.field(trade.SceneField.IMPORTER_STATE) pointer = scene.field(trade.SceneField.IMPORTER_STATE)
mutable_pointer = scene.mutable_field(trade.SceneField.IMPORTER_STATE) mutable_pointer = scene.mutable_field(trade.SceneField.IMPORTER_STATE)

10
src/python/magnum/trade.cpp

@ -772,11 +772,12 @@ void trade(py::module_& m) {
/* AbstractImporter depends on this */ /* AbstractImporter depends on this */
py::module_::import("corrade.pluginmanager"); py::module_::import("corrade.pluginmanager");
py::enum_<Trade::DataFlag> dataFlag{m, "DataFlag", "Data flag"}; py::enum_<Trade::DataFlag> dataFlag{m, "DataFlags", "Data flags"};
dataFlag dataFlag
.value("OWNED", Trade::DataFlag::Owned) .value("OWNED", Trade::DataFlag::Owned)
.value("EXTERNALLY_OWNED", Trade::DataFlag::ExternallyOwned) .value("EXTERNALLY_OWNED", Trade::DataFlag::ExternallyOwned)
.value("MUTABLE", Trade::DataFlag::Mutable); .value("MUTABLE", Trade::DataFlag::Mutable)
.value("NONE", Trade::DataFlag{});
corrade::enumOperators(dataFlag); corrade::enumOperators(dataFlag);
py::enum_<Trade::MeshAttribute> meshAttribute{m, "MeshAttribute", "Mesh attribute name"}; py::enum_<Trade::MeshAttribute> meshAttribute{m, "MeshAttribute", "Mesh attribute name"};
@ -1154,12 +1155,13 @@ void trade(py::module_& m) {
.value("STRING_RANGE_NULL_TERMINATED16", Trade::SceneFieldType::StringRangeNullTerminated16) .value("STRING_RANGE_NULL_TERMINATED16", Trade::SceneFieldType::StringRangeNullTerminated16)
.value("STRING_RANGE_NULL_TERMINATED64", Trade::SceneFieldType::StringRangeNullTerminated64); .value("STRING_RANGE_NULL_TERMINATED64", Trade::SceneFieldType::StringRangeNullTerminated64);
py::enum_<Trade::SceneFieldFlag> sceneFieldFlag{m, "SceneFieldFlag", "Scene field flag"}; py::enum_<Trade::SceneFieldFlag> sceneFieldFlag{m, "SceneFieldFlags", "Scene field flags"};
sceneFieldFlag sceneFieldFlag
.value("OFFSET_ONLY", Trade::SceneFieldFlag::OffsetOnly) .value("OFFSET_ONLY", Trade::SceneFieldFlag::OffsetOnly)
.value("ORDERED_MAPPING", Trade::SceneFieldFlag::OrderedMapping) .value("ORDERED_MAPPING", Trade::SceneFieldFlag::OrderedMapping)
.value("IMPLICIT_MAPPING", Trade::SceneFieldFlag::ImplicitMapping) .value("IMPLICIT_MAPPING", Trade::SceneFieldFlag::ImplicitMapping)
.value("NULL_TERMINATED_STRING", Trade::SceneFieldFlag::NullTerminatedString); .value("NULL_TERMINATED_STRING", Trade::SceneFieldFlag::NullTerminatedString)
.value("NONE", Trade::SceneFieldFlag{});
corrade::enumOperators(sceneFieldFlag); corrade::enumOperators(sceneFieldFlag);
py::class_<Trade::SceneData>{m, "SceneData", "Scene data"} py::class_<Trade::SceneData>{m, "SceneData", "Scene data"}

Loading…
Cancel
Save