{"action":"created","issue":{"url":"https://api.github.com/repos/FugroRoames/Proj4.jl/issues/4","labels_url":"https://api.github.com/repos/FugroRoames/Proj4.jl/issues/4/labels{/name}","comments_url":"https://api.github.com/repos/FugroRoames/Proj4.jl/issues/4/comments","events_url":"https://api.github.com/repos/FugroRoames/Proj4.jl/issues/4/events","html_url":"https://github.com/FugroRoames/Proj4.jl/pull/4","id":103964545,"number":4,"title":"Code Reorganization and Geodesic Routines","user":{"login":"yeesian","id":1231079,"avatar_url":"https://avatars.githubusercontent.com/u/1231079?v=3","gravatar_id":"","url":"https://api.github.com/users/yeesian","html_url":"https://github.com/yeesian","followers_url":"https://api.github.com/users/yeesian/followers","following_url":"https://api.github.com/users/yeesian/following{/other_user}","gists_url":"https://api.github.com/users/yeesian/gists{/gist_id}","starred_url":"https://api.github.com/users/yeesian/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yeesian/subscriptions","organizations_url":"https://api.github.com/users/yeesian/orgs","repos_url":"https://api.github.com/users/yeesian/repos","events_url":"https://api.github.com/users/yeesian/events{/privacy}","received_events_url":"https://api.github.com/users/yeesian/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":21,"created_at":"2015-08-31T00:06:28Z","updated_at":"2015-09-08T00:45:16Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/FugroRoames/Proj4.jl/pulls/4","html_url":"https://github.com/FugroRoames/Proj4.jl/pull/4","diff_url":"https://github.com/FugroRoames/Proj4.jl/pull/4.diff","patch_url":"https://github.com/FugroRoames/Proj4.jl/pull/4.patch"},"body":"Here's some collected thoughts after having worked on it over the weekend,\r\n\r\n# Code Organization\r\nWe should separate the low-level C-facing routines from the high-level user-facing ones.\r\n\r\n## lower-level\r\nIn the low-level C-facing routines (`src/geodesic.jl` and `src/proj_capi.jl`), that's where\r\n\r\n- pointer arithmetic should be handled\r\n- error messages should be dealt with\r\n- C-type immutables/structs (for the express purpose of interfacing with the PROJ.4 library, rather than for users) should be defined, and dealt with locally\r\n\r\nPreferably the only function arguments that should be pointers are native objects from the proj.4 library. I've made an exception for one of the `_transform` methods, because it gets re-used alot in the other calls to `transform` (for reshaping between `Array{T,2}` and `Vector{T}`).\r\n\r\n## higher-level\r\nThe higher-level user-facing routines are organized into `src/proj_types.jl` and `src/proj_functions.jl`. This being a wrapper library, I'm biased towards\r\n- ordered arguments, rather than keyword arguments.\r\n- concretely-typed variables, rather than loosely-typed ones\r\n- surfacing decisions for the user (through function arguments/naming), rather than making decisions for them; convenience functions can be added at a later stage\r\n\r\nThis is most obviously felt in `transform`, `transform2` and `transform3` (both with and without the bang). It might feel like code-bloat, but the wider variety of function calls is actually useful for the end-user: most of the time, they would know whether or not the data they have is 2 or 3 dimensional. In such cases, they will prefer to save on the checking/branching by calling the most specific function, especially if they're doing the geodesic operations in a tight loop.\r\n\r\n# Support\r\nWe should also provide consistency in user expectation (if violated, it's always from my own wrongdoing), rather than simply documenting interfaces:\r\n\r\n   1. calls to functions with vector arguments return vector results, and calls with NxM arrays return NxM results.\r\n   2. Results should not end up mysteriously transposed/etc.\r\n   3. If a argument of a certain type is allowed in some functions, it should be allowed in all \"similar\"/reasonable functions\r\n\r\n(This has implications in the \"other thoughts section\" when it comes to tightening the range of allowable argument types.)\r\n\r\nI don't mind if someone else comes along, and submits a PR to support v0.3, but I personally don't have the energy to maintain that level of support, sorry! I have gone back to clean up all the unused functions (`geodesic2geocentric`, `latlong_from_projection`, etc) to scope it down into something I can manage. Also, I suggest we stick with Float64/EPSG/ESRI, and support more general user arguments (Int/Real/etc) only when we have the resources (and are in a good position) to. I personally think that time/effort is better spent on developing the rest of the missing functionality for working with geospatial data (e.g. OGR/etc) though.\r\n\r\n# Other thoughts\r\n- The semantics of `Array{T,2}` is not entirely clear-cut to me; I can imagine, for instance, that the columns (rather than the rows) might correspond to distinct points. Although it might feel more \"general\"(i.e. a vector can be \"reshaped\" into a 2-d matrix), it introduces a lot of degrees of freedom in how it might/should be interpreted. Moreover, it is weirder to support Arrays without also supporting Vectors, rather than vice versa\r\n- The length of the code is secondary to me for now; it can be shortened via parameterizing the functions, or meta-programming at a later stage\r\n- I have been rather wild and loose with my individual commits, since there was (and still is) a lot of ground to cover. I expect to change after we tag the first release, and hope it doesn't annoy you too much.\r\n- re: https://github.com/FugroRoames/Proj4.jl/issues/2#issuecomment-135942011, I work from a macbook, so I can help with building/testing for OS X, but I hope you can figure out how the build service works too, to increase the bus factor of this package."},"comment":{"url":"https://api.github.com/repos/FugroRoames/Proj4.jl/issues/comments/138399143","html_url":"https://github.com/FugroRoames/Proj4.jl/pull/4#issuecomment-138399143","issue_url":"https://api.github.com/repos/FugroRoames/Proj4.jl/issues/4","id":138399143,"user":{"login":"c42f","id":601473,"avatar_url":"https://avatars.githubusercontent.com/u/601473?v=3","gravatar_id":"","url":"https://api.github.com/users/c42f","html_url":"https://github.com/c42f","followers_url":"https://api.github.com/users/c42f/followers","following_url":"https://api.github.com/users/c42f/following{/other_user}","gists_url":"https://api.github.com/users/c42f/gists{/gist_id}","starred_url":"https://api.github.com/users/c42f/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/c42f/subscriptions","organizations_url":"https://api.github.com/users/c42f/orgs","repos_url":"https://api.github.com/users/c42f/repos","events_url":"https://api.github.com/users/c42f/events{/privacy}","received_events_url":"https://api.github.com/users/c42f/received_events","type":"User","site_admin":false},"created_at":"2015-09-08T00:45:16Z","updated_at":"2015-09-08T00:45:16Z","body":"Tests pass now!  There's a couple of niggles left which I'll note on the associated lines."}}