[Bf-python] Contrib Addon Review: io_mesh_xyz

[Campbell Barton]

Hey Clemens,

Reply inline.

On Tue, Oct 30, 2012 at 7:23 PM, Clemens Barth <barth at root-1.de> wrote:
>
>
> Thanks for your review, Campbell.
>
> Just to inform you all: There are already a couple of colleagues,
> who use this I/O in research.
>
> 1. They use Blender for modeling their
> atomic structures in the 3D scene, which is then saved as an xyz
> file. The latter is used as an input for calculations.
>
> 2. Blender is certainly used for creating exciting, high-quality artwork,
> and some of them are going to be publish soon!
>
> For instance, may be you have seen our recent cover
> picture (http://clemens-barth.root-1.de/News/AdvMater_Suzuki.jpeg).
> Very recently we published an other article and used Blender again
> (http://ej.iop.org/images/1367-2630/14/10/103037/Full/nj441116f5_online.jpg).
>
> Note: Since the xyz addon still needs to be downloaded from my own server
> I have automatically the statistics: The addon is downloaded 2 to 3 times a
> day!
>
> Blender is absolutely fantastic and beats everything that is doing something
> similar in 3D. We urgently need to include this XYZ IO into trunc because
> the installation is then much easier! However, we have to take care about
> certain simplifications that should be included into the IO and that have to
> be convenient
> for, in particular, researchers (experimentalists, theorists, physics,
> chemistry,
> biology, etc.).


I dont totally agree here, If you want to have a highly specialized
tool written for your own use,
which is quite different to other Blender tools, there is nothing
wrong with that.

While reviewing your addon I was wondering if perhaps its better this
addon not go into trunk because it looks like you have managed to put
together something that works for you - it aims to be more a complete
solution for multiple tasks. For tools in blender we mostly try to
avoid duplication of effort and have them do only a few things well.


> Being import/exporter this addon can be included in blender just to
> support the format, but it does some things differently to most other
> IO scripts.
>
> Well, this is exactly the story we once had with the PDB I/O, you remember
> last year? :-)
> The panel is very useful for changing some prefs of loaded atom structures.
> If we
> remove them, we loose easy functionality!

I see where your coming from, and not saying you _should_ make this
change, but ideally IMHO, there would be an addon for import/export
and thats all it would do,
then if you wanted to have some panel with a bunch of various tools
this could be an addon distributed separately.

I realize in practice this is more effort to integrate.

> Enabling the addon, adds a panel in the toolbar, under the "Operator" panel.
> Addons currenly don't have a good place to store preferences,
>
> To my opinion, this should be changed in future: Finding a better place for
> panels. But so far,
> things also work quite well ... it is a minor problem.
>
>  but
> think this isnt a good way to work around it.
>
> Well, all my colleagues and me really need this panel daily! We beg you:
> don't remove the panel. :-)

of course not, you can keep your workflow - at this point we need to
see whats reasonable to accept into trunk though.

> This panel has settings for loading frames, I think these should be in
> the importer panel.
>
> You mean into this file dialog?

yes.

> Oh, no ... the reason is the following: When
> a structure is loaded, only the first frame is visible. Sometimes, xyz files
> may
> contain more than >1000 frames, which you sometimes don't want to see.
> So, this is why there is a 'skip frames' option. Furthermore, there is the
> option to
> put other frames in between two frames to smoothen the movements
> (frame/key).
> After having chosen the 2 prefs values, you load the frames, which may take
> some
> time.
>
> Finding out the best parameters is just done by try/error in the scene
> (playing around):
> you try out one set of parameters and if this is not good, you delete the
> keys, change the
> parameters and restart. This is quite convenient and easy to do.
>
>
>
> So I'd rather have this panel removed, settings moved into import or export.
> any settings that you may want to re-use can be stored as operator presets.
>
> As last year, we have to negotiate ... .  :-) BUT: I agree to remove these
> two
> buttons that are for the rendering of the scene.

Feel free to make a small scripts that keep this functionality if you
find it useful.

> This script needs a code-review too, some of the things it does are a
> bit unconventional for blender/python scripts.
>
> Yeah, it probably is. :-)   But, as I said, it is somewhat designed for us
> researchers to modify the loaded structure in an easy way.
>
>
> Okay, I have seen your comments to the code
> (http://codereview.appspot.com/6815052/diff/1/io_mesh_xyz/__init__.py#newcode98).
> But before I start, I would like to hear your all opinions ...
>

For now, if you have time for this is best to make the obvious
improvements suggested in the code review and leave areas you are
uncertain about.

We don't have to be totally strict about where addons put panels and
knit picks, but think its worth attempting to make addons fit in with
blender and not overload addons with too much functionality.

> Cheers,
> Belndphys.
>
>
>
>
>
> _______________________________________________
> Bf-python mailing list
> Bf-python at blender.org
> http://lists.blender.org/mailman/listinfo/bf-python
>



-- 
- Campbell