PEP8

[rowanc1]

Because I know that we should.

This could probably help ease us in:
http://propertiespy.readthedocs.io/en/latest/content/renamed.html#renamed-property

I think that there are a number of handy properties that people use quite often that would need some thought:

nC, nF, vnE etc. I think true PEP8 doesn't even like the 2 letter variable names? For some of this stuff that is used constantly, I think that is fine?

faceDiv --> face_div

Oh boy.

fatiando/fatiando#278

Thoughts @lheagy?

[rowanc1]

Some utilities that could help us from going insane:

https://pypi.python.org/pypi/autopep8
https://github.com/spulec/pep8ify

Doesn't look like these rename functions for you. Probably a good thing.

[jcapriot]

I have always been curious why you guys never followed convention for naming things pep8 style.

I am in favor of this.

[rowanc1]

This package was my first exposure to the Python language. I was young, naive, and fancy free.

[lheagy]

Oh man, there are elements that could be painful... For commonly used things, nC, etc I would be hesitant. There is a of research code upstream that will break. That said, as long as people are given sufficient time and we use something like renamed properties, it is do-able. Being in-line with pep8 would be good.

[leouieda]

This package was my first exposure to the Python language. I was young, naive, and fancy free.

@rowanc1 I've been there :) This got implemented in Fatiando right after Scipy 2014 in fatiando/fatiando#115

I used autopep8 but did it one file at a time to check that all tests and scripts were still working. I wouldn't recommend running it on the entire library at once. A good way to start to put the pep8 check into a CI system and enforce it for new code.

I recently started running flake8 in TravisCI to catch any incompatibilities (fatiando/fatiando#382). Another useful tool is pylint with the --py3k argument to make sure new code is Python 3 compatible (mostly).

@lheagy I've been very liberal with breaking backward compatibility in Fatiando. I should actually make it more clear in the docs about that. One thing that mitigates this is to always record and cite the version number in research projects. The code there will still work and produce the results from the paper, just not using the latest Fatiando. For example, I was working on this paper and this one in parallel and they both use different incompatible versions. But they both still work.

[lheagy]

Thanks @leouieda, keeping track of versions is a very good point. This is where making use of a mailing-list and making sure release notes are easily found from the docs will be important.

[lheagy]

A few naming thoughts on the base class

• nC --> ncells, similarly nF --> nfaces, nE --> nedges
• nEx --> nedges_x, etc
• vnC --> vec_ncells, or vector_ncells ?
• x0 stays? or x0 --> origin

[lheagy]

Ok, so here is a bit more of a comprehensive list of suggestions, starting at the basemesh (cc @rowanc1, @dougoldenburg, @simpeg/simpeg-developers). These are just a first pass at suggestions - please add your thoughts!

Thoughts on naming conventions

These are very open to suggestions! They are some of my initial thoughts and might help give an idea where the below suggestions came from.

• avoid shorthand, spell out the whole word
• start the name of similar properties / methods with the same first word so that they tab-complete together (this leads to thinking about the more general part of the name first, then get more specific).
  • for example: n_edges_x is better than n_x_edges as the former will be grouped in the tab completion with n_edges_y and n_edges_z
  • similarly, boundary_indices_cells is better than cell_boundary_indices as we also have boundary_indices_faces
• if there are two different ways of looking at a piece of information, try and keep the names close together
  • for example n_edges_vector is better than vector_n_edges as we will see n_edges and n_edges_vector grouped closely together in the tab completion.

Suggested Changes in BaseMesh.py and TensorMesh.py

I started by examining these two files as they are among the most core to the whole package.

BaseMesh

Properties

• dim --> dimension
• nC --> n_cells (or number_of_cells or number_cells)?
• nN --> n_nodes
• nEx --> n_edges_x (similar for nEy, nEz)
• vnE --> n_edges_vector (alternatively, vector_n_edges, but I think adding the 'vector' modifier at the end instead highlights the fact that this is just a different view of the counting of edges)
• nE --> n_edges
• nF --> n_faces

Methods

• projectFaceVector(self, fV) --> project_face_vector(self, face_vector)
• projectEdgeVector(self, eV) --> project_edge_vector(self, edge_vector)

BaseRectangularMesh

Properties

similar extension to the renaming suggested above, eg.

• nCx --> n_cells_x (similar for y and z)
• vnC --> n_cells_vector
• vnFy --> n_faces_y_vector

Methods

• r(self, x, xType='CC', outType='CC', format='V') --> `reshape(self, x, input_grid='CC', output_grid='CC', output_format='vector') # assert output_format in ['vector', 'ndgrid'] ?

BaseTensorMesh

Properties

I am less sure about these suggestions. Please chime in!

• h --> cell_widths ?? (I am not sure on this...)
• I think x0 should stay as is?
• vectorNx --> vector_nodes_x (this is another reason for the above suggestion for counting faces, nodes, etc)
• vectorCCx --> vector_cell_centers_x
• gridCC --> grid_cell_centers
• gridFx --> grid_faces_x

Methods

• getTensor(self, key) --> tensor(self, grid_location) (should this be private? or even individual properties for each of tensor_faces_x, etc)
• isInside(self, pts, locType='N') --> `is_inside(self, points, grid_location='N')
• getInterpolationMat(self, loc, locType='CC', zerosOutside=False) --> interpolation_matrix(self, points, grid_location='CC', zeros_outside=False)

(there are also private methods, but this is less of a big-deal...)

TensorMesh

Properties

• vol --> cell_volumes (or volume_cells)
• area --> face_areas (or area_faces)
• edge --> edge_lengths (length_edges)
• faceBoundaryInd --> boundary_indices_faces (or just boundary_faces ?)
• cellBoundaryInd --> boundary_indices_cells (or just boundary_faces ?)

[lheagy]

This will be helpful: https://github.com/ambv/black

[leouieda]

I've been using it for a few months and really like the freedom of not needing to explain formatting to new users or worry about it myself. That said, I'd only recommend it if you can deal with handing control of formatting completely over to the tool. The code looks way better than with autopep8, though.