{"action":"created","issue":{"url":"https://api.github.com/repos/astropy/astropy-APEs/issues/14","repository_url":"https://api.github.com/repos/astropy/astropy-APEs","labels_url":"https://api.github.com/repos/astropy/astropy-APEs/issues/14/labels{/name}","comments_url":"https://api.github.com/repos/astropy/astropy-APEs/issues/14/comments","events_url":"https://api.github.com/repos/astropy/astropy-APEs/issues/14/events","html_url":"https://github.com/astropy/astropy-APEs/pull/14","id":167650551,"number":14,"title":"Remaking NDData to support LSST needs","user":{"login":"perrygreenfield","id":981803,"avatar_url":"https://avatars.githubusercontent.com/u/981803?v=3","gravatar_id":"","url":"https://api.github.com/users/perrygreenfield","html_url":"https://github.com/perrygreenfield","followers_url":"https://api.github.com/users/perrygreenfield/followers","following_url":"https://api.github.com/users/perrygreenfield/following{/other_user}","gists_url":"https://api.github.com/users/perrygreenfield/gists{/gist_id}","starred_url":"https://api.github.com/users/perrygreenfield/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/perrygreenfield/subscriptions","organizations_url":"https://api.github.com/users/perrygreenfield/orgs","repos_url":"https://api.github.com/users/perrygreenfield/repos","events_url":"https://api.github.com/users/perrygreenfield/events{/privacy}","received_events_url":"https://api.github.com/users/perrygreenfield/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-07-26T16:20:33Z","updated_at":"2016-07-26T21:22:16Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/astropy/astropy-APEs/pulls/14","html_url":"https://github.com/astropy/astropy-APEs/pull/14","diff_url":"https://github.com/astropy/astropy-APEs/pull/14.diff","patch_url":"https://github.com/astropy/astropy-APEs/pull/14.patch"},"body":"This is a first draft of an APE to support the issues raised by LSST and the subsequent email discussions regarding that"},"comment":{"url":"https://api.github.com/repos/astropy/astropy-APEs/issues/comments/235409576","html_url":"https://github.com/astropy/astropy-APEs/pull/14#issuecomment-235409576","issue_url":"https://api.github.com/repos/astropy/astropy-APEs/issues/14","id":235409576,"user":{"login":"MSeifert04","id":14200878,"avatar_url":"https://avatars.githubusercontent.com/u/14200878?v=3","gravatar_id":"","url":"https://api.github.com/users/MSeifert04","html_url":"https://github.com/MSeifert04","followers_url":"https://api.github.com/users/MSeifert04/followers","following_url":"https://api.github.com/users/MSeifert04/following{/other_user}","gists_url":"https://api.github.com/users/MSeifert04/gists{/gist_id}","starred_url":"https://api.github.com/users/MSeifert04/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MSeifert04/subscriptions","organizations_url":"https://api.github.com/users/MSeifert04/orgs","repos_url":"https://api.github.com/users/MSeifert04/repos","events_url":"https://api.github.com/users/MSeifert04/events{/privacy}","received_events_url":"https://api.github.com/users/MSeifert04/received_events","type":"User","site_admin":false},"created_at":"2016-07-26T21:22:16Z","updated_at":"2016-07-26T21:22:16Z","body":"I'm not sure where the discussion should be.\r\n\r\nFirst: I agree on many points of your APE but I don't really like the proposed implementation.\r\n\r\nLet me summarize the current standard:\r\n\r\n- `NDDataBase` - Interface definition of \"common\" attributes\r\n- `NDData` - Container based on `NDDataBase`, the restrictions here are: `data`: numpy-array-like, `meta`: dictionary-like, `uncertainty`: NDUncertainty-like\r\n- `NDDataRef` - `NDData` with specified slicing, arithmetics and io-interface.\r\n\r\nSo if reworking `NDData` (or `NDArr`) to some \"working\" implementation would promote the class from a simple container. That's not bad but worth mentioning because it does not only break backwards compatibility (possibly) but also alter the basic data structure.\r\n\r\nAdditionally the current `NDData` interface would be totally (or mostly) compatible with your suggestions except for the two-layered structure:\r\n\r\n- `bitmasks` as `mask`: No problem there. It wouldn't work with \"NDArithmeticMixin\" (by default) but the current implementation would allow with a minimum of tweaking, it may be as easy as alter one default parameter (\"handle_mask\"). for \"NDData\" the mask is designed as property but there is no reason why a property shouldn't return a callable. So there is currently no problem with just using your \"mask\" class. Besides this there is a flags attribute in NDDataArray which could be reinterpreted as the bitmask without seriously altering mask at all.\r\n- `offset`: Slicing is implemented in \"NDSlicingMixin\". It's to some extend customizable and adding a \"offset\" wouldn't be any problem. It doesn't support all possible numpy indexing ways because the `astropy.WCS` doesn't support them. But it also is highly customizable so there is actually no problem instead of slicing the wcs to alter some property (like the proposed offset).\r\n- `variance`: I actually have a working implementation of Variance-uncertainty based on `NDUncertainty` which supports the proposed operations (+, -, /, *). So it's no problem to implement something like that within the current framework.\r\n- ExposureInfo: I think this would be far too specific for `NDData`. You assume that the `NDData` is used for exposures is very strict in my opinion.\r\n- Two-layer-class-structure: Could you clarify _what_ would be seperated, especially _when_ and _why_ this would be needed or not achievable with the current implementation?\r\n\r\nSorry this was so negative. I think a lot of your APE makes sense, for example the bitmask, exposureinfo and variance uncertainty would be really great additions. But I somehow disagree on the proposed implementation. I think these could also be integrated without changing (much) of `NDData` itself. Altough I have to admit that I'm probably biased on these points because I did change a lot in NDData :smile: \r\n\r\nJust to cross-link it here: @crawfordsm already mentioned some or even all of these points in https://groups.google.com/forum/#!topic/astropy-dev/I_co2botSNw "}}
{"actor":{"display_login":"MSeifert04"}}