faitdivers / pyao

PyAO - Adaptive Optics simulation package
6 stars 0 forks source link

Centroid #63

Closed herminarto closed 10 years ago

herminarto commented 10 years ago

Notes:

Calculate the centroid and slopes for all apertures: The structure of the return on the function is a row vector of slopes: [s_x(0,0),.,s_x(0,N),.,s_x(M,0),.,s_x(M,N),s_y(0,0),.,s_y(0,N),.,s_y(M,0),.,s_y(M,N)]

Centroid calculation using thresholding method.

forcaeluz commented 10 years ago

Nice coding! I would only suggest to not create a different file for each function you make. Your centroid function is also quite big. Think about how to split it into sub-functions, to increase readability.

Also, merge the master into your branch, so that we can merge without conflicts. (Attention here: Merge master into yours, not your branch into master yet)

faitdivers commented 10 years ago

@herminarto , please note that there are still some conflicts and that it is still not possible to merge the branch automatically.

forcaeluz commented 10 years ago

As commented before, I would prefer to see the functions in one file, as they are still specific to the centroid module. That keeps the things easier to maintain, as I don't need to open multiple files to work on 1 module only.

faitdivers commented 10 years ago

@herminarto please note that the code can not be automatically merged, now.

faitdivers commented 10 years ago

@herminarto please solve the conflicts so that we can merge this branch into the master.

herminarto commented 10 years ago

@forcaeluz , I have fixed the conflicts, merge the master into branch centroid, and push the branch. But why github still can not automatically merge this pull request?

-- Edit -- Sorry, my mistake. It can be automatically merge now. Can you check it?