Hi James,

Congratulations on your first commit! Without wanting to nitpick too much there were a couple of things that I would like to comment on:

Personally I would have preferred the function name as_miller_arrays_dict(), as this would make it a lot clearer (at least to me) what to expect from the function before looking at the documentation. At first I was confused because the 'map' part of the name made me think that it was doing something to do with electron density maps (especially given the context of miller arrays). Replacing 'map' with 'dict' in the name would avoid any potential for confusion.

Also, I would prefer the function arguments to be spelled out explicitly without using *args and **kwds, so that I can figure out what arguments it takes without having to read through the function source code to figure out that internally you are calling self.as_miller_arrays(), and then have to go and look up the function arguments for that. My IDE (WingIDE) shows me the function signature in its source assistant and also provides code completion for function arguments and keywords. However these won't be of much use when functions only list *args and **kwds.

Cheers,

Richard


On 17 April 2013 21:57, James Stroud <xtald00d@gmail.com> wrote:
Hello All,

Since this is my first commit, I just want to make sure that I have done these things in the appropriate way.

Here's what I did:

1. Added a wiki page: https://sourceforge.net/p/cctbx/wiki/Subversion%20set%20up/
2. Added the method as_miller_arrays_map() to iotbx_mtz_ext.object, which is declared in iotbx/mtz/__init__.py.
3. Added a test for this functionality in the file iotbx/mtz/tst_miller_map.py

Please let me know if the manner in which I did anything could use improvement.

James

_______________________________________________
cctbxbb mailing list
cctbxbb@phenix-online.org
http://phenix-online.org/mailman/listinfo/cctbxbb