[cctbxbb] First svn commit

[Richard Gildea]

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 at 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 at phenix-online.org
> http://phenix-online.org/mailman/listinfo/cctbxbb
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://phenix-online.org/pipermail/cctbxbb/attachments/20130417/e790d7b3/attachment.htm>