{"action":"created","issue":{"url":"https://api.github.com/repos/PyAbel/PyAbel/issues/36","labels_url":"https://api.github.com/repos/PyAbel/PyAbel/issues/36/labels{/name}","comments_url":"https://api.github.com/repos/PyAbel/PyAbel/issues/36/comments","events_url":"https://api.github.com/repos/PyAbel/PyAbel/issues/36/events","html_url":"https://github.com/PyAbel/PyAbel/issues/36","id":119103793,"number":36,"title":"Discussion on a consistant naming scheme for different implementations","user":{"login":"rth","id":630936,"avatar_url":"https://avatars.githubusercontent.com/u/630936?v=3","gravatar_id":"","url":"https://api.github.com/users/rth","html_url":"https://github.com/rth","followers_url":"https://api.github.com/users/rth/followers","following_url":"https://api.github.com/users/rth/following{/other_user}","gists_url":"https://api.github.com/users/rth/gists{/gist_id}","starred_url":"https://api.github.com/users/rth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rth/subscriptions","organizations_url":"https://api.github.com/users/rth/orgs","repos_url":"https://api.github.com/users/rth/repos","events_url":"https://api.github.com/users/rth/events{/privacy}","received_events_url":"https://api.github.com/users/rth/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/PyAbel/PyAbel/labels/discussion","name":"discussion","color":"fef2c0"}],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/PyAbel/PyAbel/milestones/2","html_url":"https://github.com/PyAbel/PyAbel/milestones/Version%200.7","labels_url":"https://api.github.com/repos/PyAbel/PyAbel/milestones/2/labels","id":1423032,"number":2,"title":"Version 0.7","description":"Adding a few other inverse Abel implementations. Converge to a consistent user interface.","creator":{"login":"rth","id":630936,"avatar_url":"https://avatars.githubusercontent.com/u/630936?v=3","gravatar_id":"","url":"https://api.github.com/users/rth","html_url":"https://github.com/rth","followers_url":"https://api.github.com/users/rth/followers","following_url":"https://api.github.com/users/rth/following{/other_user}","gists_url":"https://api.github.com/users/rth/gists{/gist_id}","starred_url":"https://api.github.com/users/rth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rth/subscriptions","organizations_url":"https://api.github.com/users/rth/orgs","repos_url":"https://api.github.com/users/rth/repos","events_url":"https://api.github.com/users/rth/events{/privacy}","received_events_url":"https://api.github.com/users/rth/received_events","type":"User","site_admin":false},"open_issues":10,"closed_issues":1,"state":"open","created_at":"2015-11-21T11:17:04Z","updated_at":"2015-12-01T17:01:21Z","due_on":null,"closed_at":null},"comments":3,"created_at":"2015-11-26T21:33:19Z","updated_at":"2015-12-01T17:48:58Z","closed_at":null,"body":"Since there is an increasing number of implementations that will be merged into PyAbel in the future (including issues #27 , #35 , #24 , #30 , #23 ), it would be good to agree on some consistent naming scheme for different functions. This issue aims to discuss how we will handle this in PyAbel >= 0.7, and is a continuation of the discussion in issue #14 . \r\n\r\nHere is a first working draft, closely based on the way different minimization solvers are handled in [`scipy.optimize`](https://docs.scipy.org/doc/scipy-0.16.0/reference/optimize.html). So we could, for instance, define general wrappers for the forward and inverse abel functions as `abel.fabel` and `abel.iabel`,  with the following example:\r\n\r\n          import abel\r\n          abel.iabel(data, method='BASEX', options={})\r\n\r\nwhere `data` is a centered array, `method` is the implementation that should be used, and `options` a dictionary with the implementation specific options. \r\n\r\nThe specific naming for different implementations, located under `abel/<implementation>.py`  could be then,\r\n -  `abel.fabel_<implemenation>`  :  forward transform (when defined)\r\n -  `abel.iabel_<implemenation>` :  inverse implementation (when defined)\r\n -  `abel._bs_<implementation>` :  function that generates  the basis sets (if necessary)\r\n -  `abel.bs_<implementation>_cached` : function that loads the basis sets from disk, and generates them if they are not found (if necessary).\r\n\r\nThe centering function and the velocity  maps are imported as presently from under `abel.tools`.\r\n\r\nBelow is the list of present and planned implementations, with a list of features, including,\r\n  - **Name** : the name of the method\r\n  - **Version**: version of PyAbel when it is planned to be added\r\n  - **Symmetry**: the symmetry of the reconstruction,\r\n      - `None`: the parts for the image for `r>0` and `r<0` (and for every `z` value) are reconstructed separately, assuming that they represent and object with a cylindrical symmetry\r\n      -  `r`:   The reconstruction verifies `f(r,z) = f(-r, z)`\r\n      - `r`, `z`: The reconstruction verifies `f(r,z) = f(-r, z)` and `f(r,-z) = f(r, z)`  (polar symmetry?) \r\n  - **Basis sets:** implementation uses pre calculated basis sets\r\n  - **`dr = cst.`:**  implementation requires uniform sampling of the image along the `r` axis. \r\n\r\nand naming conventions,\r\n \r\n   - **`method`**: the value of the argument given to the wrapper `abel.abel(... method=method)`\r\n   - **Files** : files under `abel/` where the code is located\r\n   - **Core functions**: low level core functions that perform the transform of the centered signal\r\n   - **Basis set functions**: low level core functions that generate basis sets (if that is necessary)\r\n   - **Wrappers**: wrappers that perform centering, (optionally) basis set generation, and the transform\r\n \r\n### Forward Abel implementations \r\n\r\n##### Features\r\n| Name                      | Version    | Symmetry | Basis sets | `dr = cst.` | Comments |\r\n|:-------------------------:|:----------:|:--------:|:----------:|:-----------:|:--------:|\r\n| Fourier Hankel            | `>=v0.7`   | `None`      |            |   ✔         |          |\r\n| Direct integration        | `v0.7`     | `None`      |            |             |          |\r\n\r\n##### Naming conventions\r\n\r\n| Name                      | `method`   | Files      | Core functions  | Basis set functions    | Wrappers |\r\n|:-------------------------:|:----------:|:----------:|:---------------:|:-----------:|:--------:|\r\n| Fourier Hankel            | `FH`       | `fh.py`    | `fabel_fh`      |    None         |  None    |\r\n| Direct integration        | `direct`   | `direct.py`| `fabel_direct`  |    None         |  None    |\r\n\r\n### Inverse Abel implementations \r\n\r\n##### Features\r\n\r\n| Name                      | Version    | Symmetry | Basis sets | `dr = cst.` | Comments |\r\n|:-------------------------:|:----------:|:--------:|:----------:|:-----------:|:--------:|\r\n| BASEX                     | `v0.6`     | `r`, `z` | ✔          |   ✔         |          |\r\n| BASEX                     | `v0.7`     | `r`      | ✔          |   ✔         |          |\r\n| Fourier Hankel            | `>=v0.7`   | `None`?  |            |   ✔         |          |\r\n| Direct integration        | `v0.7`     | `None`   |            |             |          |\r\n| Hansen & Law              | `v0.7`     | `r`, `z` |            |   ✔         |          |\r\n| Polar Onion Peeling (POP) | `v0.8`     | `r`, `z` | ✔          |   ✔         |          |\r\n| Onion Peeling (OP)        | `>=v0.7`   | `r`      |            |   ✔         |          |\r\n\r\n##### Naming conventions\r\n\r\n| Name                      | `method`   | Files      | Core functions  | Basis set functions    | Comments |\r\n|:-------------------------:|:----------:|:----------:|:---------------:|:-----------:|:--------:|\r\n| BASEX                     | `BASEX`    | `basex.py` | `iabel_basex(..., vertical_symmetry=True)` |   `_bs_basex`, `bs_basex_cached`,         |     ???     |\r\n| BASEX without up/down sym| `BASEX_asym`    | `basex.py` | `iabel_basex(..., vertical_symmetry=False)`|   `_bs_basex_asim`, `bs_basex_asim_cached`     |    ???      |\r\n| Fourier Hankel            | `FH`       | `fh.py`    | `iabel_fh`      |   None          |  None    |\r\n| Direct integration        | `direct`   | `direct.py`| `iabel_direct`  |   None          |  None    |\r\n| Hansen & Law              | `HansenLaw`| `hansenlaw.py`| `iabel_hansenlaw`|   None      |  None    |\r\n| Polar Onion Peeling (POP) | `POP`      | `pop.py`      | `iabel_pop`     |   `_bs_pop`, `bs_pop_cached`   |          |\r\n| Onion Peeling (OP)        | `OP`       | `op.py`       | `iabel_op`      |   None       |          |\r\n\r\n@DanHickstein This also addresses your questions regarding function names from PR #28 and #22, I didn't respond to at the time. \r\n\r\nSo  this is a first draft, mostly to have some basis for this discussion. Let me know what you think about it. \r\n\r\nIn particular, this draft has following issues:\r\n   1. **BASEX**: I'm struggling a bit for the `method` names (and short functions names) for BASEX without up/down symmetry (i.e. pyroBASEX). \r\n   2. `import abel` and `from abel import abel` would result in the same name, which is confusing\r\n   2. I'm not sure what would be the best way of implementing wrappers that do centering/basis set generation and transform in one line; as proposed by @DanHickstein . I don't think it would be practical to implement it for every single transform. Maybe handle it at the level of a general wrapper such as for instance `abel.center_iabel_vmi(..., method='BASEX')`, so it is not confused with the function that only does the Abel transform? We can also keep the  `BASEX` function the way it is for backward compatibility, I'm just wondering what would be the best way of handling it in the future...  \r\n\r\nA few additional notes:\r\n   - It is debatable whether `from abel import abel_basex` is better then `from abel.basex import abel_basex`. On the other hand, I'm a bit against `from abel.basex import abel` (or `import transform`), as functions should have a unique name within the module (as it is for instance the case within scipy). \r\n   - `basis_sets_` something, makes really long function names, so one way of doing it could be to abbreviate it to `bs` as I proposed here, but maybe some intermediate solution would be doable.\r\n\r\nFor other naming proposition, I guess the only other would be the Python naming conventions for [modules](https://www.python.org/dev/peps/pep-0008/#package-and-module-names) and [function](https://www.python.org/dev/peps/pep-0008/#function-names).\r\n  \r\nPlease let me know your thoughts on this. You should also be able to edit this comment (and in particular the tables above) at any point of the discussion so we can converge to a naming system that everybody agrees with. \r\n\r\n   \r\n "},"comment":{"url":"https://api.github.com/repos/PyAbel/PyAbel/issues/comments/161045613","html_url":"https://github.com/PyAbel/PyAbel/issues/36#issuecomment-161045613","issue_url":"https://api.github.com/repos/PyAbel/PyAbel/issues/36","id":161045613,"user":{"login":"DanHickstein","id":1107796,"avatar_url":"https://avatars.githubusercontent.com/u/1107796?v=3","gravatar_id":"","url":"https://api.github.com/users/DanHickstein","html_url":"https://github.com/DanHickstein","followers_url":"https://api.github.com/users/DanHickstein/followers","following_url":"https://api.github.com/users/DanHickstein/following{/other_user}","gists_url":"https://api.github.com/users/DanHickstein/gists{/gist_id}","starred_url":"https://api.github.com/users/DanHickstein/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DanHickstein/subscriptions","organizations_url":"https://api.github.com/users/DanHickstein/orgs","repos_url":"https://api.github.com/users/DanHickstein/repos","events_url":"https://api.github.com/users/DanHickstein/events{/privacy}","received_events_url":"https://api.github.com/users/DanHickstein/received_events","type":"User","site_admin":false},"created_at":"2015-12-01T17:48:58Z","updated_at":"2015-12-01T17:48:58Z","body":"Yup, the three line solution is fine. It makes it more clear the user what steps are being performed, and it will make it clear what type of angular integration is happening. \r\n\r\nI also like the idea of returning an object, where the user can get the transform with ``result.transform`` or something like this. \r\n\r\nThis opens even more possibilities. For example, in BASEX (and I assume most other methods), it is also possible to calculate the projected image using the same coefficients matrix that calculates the slice (transform). Users might want to use this image in order to compare it with the raw data and calculate the residuals in order to get some idea of how well the transform is working. \r\n\r\nIn addition, I can envision the various iabel transforms returning \r\n```\r\nresults.transform\r\nresults.projection\r\nresults.radial_intensity\r\nresults.anisotropy\r\n```\r\nand maybe others. So, having ``abel.iabel`` return an object (or, I guess dict) seems like the only way to go to ensure consistent behavior. "}}