Details

Reviewers

Summary

Create test/exp object in masses across different files.

-create a test/exp object pair
-put it in a collection
-give a path of your choice
-supports duplicating of objects
-supports creating objects in multiple files
-added support for creating vertex groups on an object

To run the file, the following command is used :
blender -b --python path/to/demo_file_generation.py

Diff Detail

Repository

rB Blender

Branch

gen-script (branched from master)

Build Status

Buildable 9727
Build 9727: arc lint + arc unit

Event Timeline

Himanshi Kalra (calra) requested review of this revision.Aug 21 2020, 9:07 PM

Himanshi Kalra (calra) created this revision.

Harbormaster completed remote builds in B9727: Diff 28045.Aug 21 2020, 9:07 PM

Himanshi Kalra (calra) edited the summary of this revision. (Show Details)Aug 21 2020, 9:07 PM

I miss the context for this patch, what kinds of issues is it intended to find exactly?

Campbell Barton (campbellbarton) added inline comments.Aug 22 2020, 11:02 AM

tests/python/demo_file_generator.py
41	Generated files shouldn't use hard coded path.
tests/python/modules/object_generator.py
113–120	Assigning `scene_name` here seems redundant, assigning `scene = bpy.context.scene` is enough, no need for lookups to `bpy.data.scenes` after that.
146	Use `os.path.join`.

Kenzie (kenziemac130) awarded a token.Aug 22 2020, 11:32 AM

To summarize the comments:

Each class/function should have a single responsibility
GenObject should take care of object and file creation, whereas TestObject only holds creation specs

tests/python/demo_file_generator.py
34	having duplicates means the given name by the user won't actually match the one created. So the user has to check for names manually before writing the tests. Is this desired?
tests/python/modules/object_generator.py
28–31	This class does way more than just holding specs. It creates the test object with an offset too. I think this is too much. Object creation should be the responsibility of `GenObject`, because only `GenObject` knows about all the test objects at the same time and therefore can handle the offset between objects much better.
79	`list_of_added_objects` is a parameter that `GenObject` should take and and not `TestObject` itself, because as mentioned above, `GenObject` should be responsible for coordinating `TestObject` class instances
109	this looks like a cleanup function that should be called in the destructor of `GenObject`
121–129	I think you should take care of correct linking where you create the object. It really seems weird that a `TestObject` instance cleans up or unlinks other object it might not know about.
123	This looks like the wrong place for such a message. It should be shown where the object was actually created and not when it's about to get removed from the default collection
132	`GenObject` -> `GenerateTestObjects`
134	This class also does more than that, but it's fine. All test and file creation should happen in this class
162–164	`_check_for_valid_path()` should really only check if the given path is valid. Hiding objects should be the responsibility of another function.

This revision now requires changes to proceed.Aug 22 2020, 11:50 AM

In D8674#211464, @Campbell Barton (campbellbarton) wrote:

I miss the context for this patch, what kinds of issues is it intended to find exactly?

@Campbell Barton (campbellbarton) The context for the patch is D8507, this patch will help in creating test objects(mesh tests) using a script, and then we can write tests using D8507

WIP: add hide function
Addressed comments, redesigned class structures

Harbormaster completed remote builds in B12196: Diff 32784.Jan 14 2021, 5:31 PM

Updated to a newer version

Harbormaster completed remote builds in B12198: Diff 32788.Jan 14 2021, 5:46 PM

Himanshi Kalra (calra) marked 8 inline comments as done.Jan 14 2021, 5:50 PM

Himanshi Kalra (calra) added inline comments.

tests/python/demo_file_generator.py
34	the user still would have to check the names anyway to get an idea, the create duplicates parameter is optional. With test object and exp object names starting from `0`
tests/python/modules/object_generator.py
109	Not very clear how a destructor would be called in here.

Himanshi Kalra (calra) added inline comments.Jan 14 2021, 5:52 PM

tests/python/demo_file_generator.py
41	Didn't understand, if its a utiltity. User can type their own paths. This is just an example on how to use it. Can also be passed an argument, but it looks fine here too IMO.

Himanshi Kalra (calra) edited the summary of this revision. (Show Details)Jan 14 2021, 6:38 PM

Added support for vertex group
added an example in demo file

Harbormaster completed remote builds in B12225: Diff 32835.Jan 16 2021, 6:26 PM

Himanshi Kalra (calra) edited the summary of this revision. (Show Details)Jan 16 2021, 6:28 PM

Habib Gahbiche (zazizizou) added inline comments.Jan 16 2021, 7:22 PM

tests/python/modules/object_generator.py
110–111	Can you explain the responsibility of the class `GenerateTestObject`? Does it not necessarily generate test objects? The original idea was to define a class that specifies how an object should be created, e.g. `TestObject` and another one that actually creates the object, i.e. `GenerateTestObject`, so any property of the test object (selection, vertex group, etc..) should be stored in `TestObject`

Himanshi Kalra (calra) added inline comments.Jan 16 2021, 8:07 PM

tests/python/modules/object_generator.py
110–111	Yes but it also generates additional supporting for test objects. Hmm, I see. Giving Vertex Group, a separate class would allow for less dependence. Adding vertex groups to already existing objects. Also I can't recall what exactly is meant by `selection` here? (Is it related to selecting vertices in a vertex group?) Also having so many optional parameters would complicate the interface, no?

Creating vertex group is a property of TestObject class
Example showing usage of interface

Harbormaster completed remote builds in B12226: Diff 32837.Jan 16 2021, 8:39 PM

I didn't look at the rest in detail, but it looks like the script has enough features to evaluate whether it's useful or not. You can try to generate many test cases for operators and modifiers and see if it saves you time or not.

tests/python/demo_file_generator.py
35	Prefer no abbreviations in interface, so `vertex_group_name` instead of `vg_name`
tests/python/modules/object_generator.py
34	Why not use the a similar interface to `OperatorSpec` for selection?
58	Why? you can always extract the directory from the full path and therefore simplify the interface
85	Prefer more specific exceptions, e.g. `NotADirectoryError` or `FileNotFoundError`
107	This is not helpful. It's better to re-raise the original exception.
110–111	Selection is useful for operators because they operate on selection and not on the whole mesh. For the interface making most of the parameters optional should solve it. See how `bpy.ops.wm.save_as_mainfile` has so many parameters and it's still easy to use because you usually only set those that you need.
119	outdated doc

Test Object Generator Script Revamped(GSoc 2020)
Needs ReviewPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 28045

tests/python/demo_file_generator.py

tests/python/modules/object_generator.py

Test Object Generator Script Revamped(GSoc 2020)Needs ReviewPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 28045

tests/python/demo_file_generator.py

tests/python/modules/object_generator.py

Test Object Generator Script Revamped(GSoc 2020)
Needs ReviewPublic
Actions

Revision Contents
Changeset List