search

fact-project / photon_stream

Explore the novel photon stream, based on the single photon extractor
6 stars 2 forks source link

Rename modules to snake_case #49

Closed KevSed closed 6 years ago

KevSed commented 6 years ago

Also point_cloud renamed to 'xyt'. 'test_muon_detection' is failing, but I can't figure out why.

relleums commented 6 years ago

Your text editor changes trailing white spaces and line endings at the end of files. Please turn this off or make a separate commit where you only change white spaces. Otherwise the git-diff is rather noisy.

relleums commented 6 years ago

The 'point_cloud' once was named 'xyt' in the beginning. This was also not clear to everybody, e.g. Kai once asked me what it is and does. I then chose the name 'point cloud', because there is a whole community with libraries for 'point cloud' manipulation out there. E.g. http://pointclouds.org/. These guys call such a data representation a 'point cloud'. Many algorithms of this 'point cloud' libraries might be of interest also for us. In any case, this also needs changes in the README where I added a discussion on the three different representations 'list-of-lists', 'image-sequence', and 'point-cloud'.

relleums commented 6 years ago

I like also to rename the modules, because I think this is recommended in the pep-8 style guide, right?

https://www.python.org/dev/peps/pep-0008/#package-and-module-names

Can we have a pull request which is only about changing the module names?

relleums commented 6 years ago

I branched and created a pull request #50 where I implement the first of your three requests, the module renaming to all-lowercase with underscores pep8

relleums commented 6 years ago

The remaining 2 changes are:

1st: Trailing white spaces. I agree, we need to tackle this. Otherwise our git diffs are noisy. Although pep8 is not strict about trailing white spaces, it still has warnings which I suggest to use. pep8 W291 trailing whitespace and pep8 W292 no newline at end of file. In general, separate commits which are only about making the code more pep8 compatible are always welcome I think.

2nd: Renaming the 'xyt' or 'point_cloud' representation of the photon_stream. Here I can only tell my story about 'xyt' also used to confuse people and that after some research I found a large community working on such data representations and calling it 'point_clouds'. Please give an argument why 'point_cloud' should be renamed again to 'xyt'

maxnoe commented 6 years ago

Please give an argument why 'point_cloud' should be renamed again to 'xyt'

I don't understand. xyt tells much more than point_cloud. Everyhting is a point-cloud. A physicist will understand the meaning of an array of shape (N, 3) with name xyt immediately.