search

SEEG-Oxford / movement

R package containing useful functions for the analysis of movement data in disease modelling and mapping
20 stars 16 forks source link

Clean up code for consistent coding style #57

Open KathrinTessella opened 9 years ago

KathrinTessella commented 9 years ago

In the following we will use the following naming convention: function names using camelCase notation arguments, objects and class names using snake_case (underscore for separating) In general, the 'dot' notation should be avoided.

Other elements of style should follow the Google style guide: https://google-styleguide.googlecode.com/svn/trunk/Rguide.xml except that function documentation should go above functions, since they’re roxygen docs.

KathrinTessella commented 8 years ago

@goldingn Do we want to use the camelCase notation also for the following functions: 'as.locationdataframe', 'is.locationdataframe', 'as.movementmatrix', 'is.movementmatrix' ?

in which case 'as.locationdataframe.data.frame' would be renamed to 'asLocationdataframe.data.frame' and as.locationdataframe.SpatialPolygonsDataFrame => asLocationdataframe.SpatialPolygonsDataFrame

goldingn commented 8 years ago

I think it should be consistent with the class name, for clarity and consistency with the S3 methods.

That said, we should rename these classes (and functions) into snake case. I.e. movement_matrix, location_dataframe, as.movement_matrix etc.

KathrinTessella commented 8 years ago

@goldingn : the package contains the function R resultasdataframe() which is not exported and not used within the package. Do we want such a functionality for the now named 'movement_predictions' object? Otherwise we should remove this function as its not used.

goldingn commented 8 years ago

yes, could you please define, document and export as.data.frame.movement_matrix, using this code but making the input a movement_matrix. This will be useful for converting the movement matrices into a form people can use.

KathrinTessella commented 8 years ago

@goldingn : to get the 'origin' and 'destination' columns for the R data.frame from the R as.data.frame.movement_matrix I require some form of location information. For matrices with column & row names, this information can be extracted from the matrices themselves, but in most cases the columns & row names are not useful.

Is it ok to take the R location_dataframe object as a 2nd argument for the method to receive the 'origin' and 'destination' information from that structure ?

goldingn commented 8 years ago

Hmmm... I would rather enforce that the row and column names are meaningful whenever creating a movement_matrix. We could then also check that these line up with the location_dataframe locations during model fitting which would be safer.

We can issue a warning or an error if something is coerced into a movement_matrix without row and column names.

KathrinTessella commented 8 years ago

Ok - I will then update the method accordingly and created a new issue (see https://github.com/SEEG-Oxford/movement/issues/99) to ensure that the column & row names are set correctly.