{"action":"created","issue":{"url":"https://api.github.com/repos/ChandraCXC/iris/issues/168","labels_url":"https://api.github.com/repos/ChandraCXC/iris/issues/168/labels{/name}","comments_url":"https://api.github.com/repos/ChandraCXC/iris/issues/168/comments","events_url":"https://api.github.com/repos/ChandraCXC/iris/issues/168/events","html_url":"https://github.com/ChandraCXC/iris/pull/168","id":113902369,"number":168,"title":"158 159 160 remove dependency on specview","user":{"login":"olaurino","id":5692321,"avatar_url":"https://avatars.githubusercontent.com/u/5692321?v=3","gravatar_id":"","url":"https://api.github.com/users/olaurino","html_url":"https://github.com/olaurino","followers_url":"https://api.github.com/users/olaurino/followers","following_url":"https://api.github.com/users/olaurino/following{/other_user}","gists_url":"https://api.github.com/users/olaurino/gists{/gist_id}","starred_url":"https://api.github.com/users/olaurino/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/olaurino/subscriptions","organizations_url":"https://api.github.com/users/olaurino/orgs","repos_url":"https://api.github.com/users/olaurino/repos","events_url":"https://api.github.com/users/olaurino/events{/privacy}","received_events_url":"https://api.github.com/users/olaurino/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/ChandraCXC/iris/labels/2%20-%20Working","name":"2 - Working","color":"CCCCCC"}],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/ChandraCXC/iris/milestones/3","html_url":"https://github.com/ChandraCXC/iris/milestones/Sprint%2011/06/15","labels_url":"https://api.github.com/repos/ChandraCXC/iris/milestones/3/labels","id":1368919,"number":3,"title":"Sprint 11/06/15","description":"","creator":{"login":"eholum","id":7966037,"avatar_url":"https://avatars.githubusercontent.com/u/7966037?v=3","gravatar_id":"","url":"https://api.github.com/users/eholum","html_url":"https://github.com/eholum","followers_url":"https://api.github.com/users/eholum/followers","following_url":"https://api.github.com/users/eholum/following{/other_user}","gists_url":"https://api.github.com/users/eholum/gists{/gist_id}","starred_url":"https://api.github.com/users/eholum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eholum/subscriptions","organizations_url":"https://api.github.com/users/eholum/orgs","repos_url":"https://api.github.com/users/eholum/repos","events_url":"https://api.github.com/users/eholum/events{/privacy}","received_events_url":"https://api.github.com/users/eholum/received_events","type":"User","site_admin":false},"open_issues":13,"closed_issues":3,"state":"open","created_at":"2015-10-21T19:08:16Z","updated_at":"2015-10-28T19:20:50Z","due_on":"2015-11-06T05:00:00Z","closed_at":null},"comments":2,"created_at":"2015-10-28T19:20:44Z","updated_at":"2015-10-29T01:18:09Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ChandraCXC/iris/pulls/168","html_url":"https://github.com/ChandraCXC/iris/pull/168","diff_url":"https://github.com/ChandraCXC/iris/pull/168.diff","patch_url":"https://github.com/ChandraCXC/iris/pull/168.patch"},"body":"Address #158, #159, #160.\r\n\r\n# Description\r\nThis PR:\r\n* removes all references and dependencies to Specview code (#159)\r\n* replaces the Units calls with a dummy replacement, decoupling the Units implementation from its API (#160)\r\n* replaces the fit attachment calls with a standalone interface, properly decoupling what was tightly coupled in the current version\r\n* as a result, some hacks have been removed (#158)\r\n* cleans up tab characters in affected classes.\r\n* Introduces a UTYPEs class with the one UTYPE that is actually used in the affected code.\r\n\r\nUnfortunately the only tests that I would expect to be broken, i.e. the ones relevant to SedStacker, are currently ignored, and we don't have tests of the Units framework that we picked off the shelf. Thus, the tests actually pass.\r\n\r\nHowever, this PR currently breaks the smoke test, and this is expected because the dummy units package does not perform any actual conversions, making the assertions in the smoke test fail.\r\n\r\nMy plan is to introduce Specview again in this very same PR, add an implementation of the new Units API based on specview so that we can introduce it again.\r\n\r\nThis PR has implications on PR #154 (Evaluate units code) as it shows the few methods that we actually use from that framework.\r\n\r\nAs for the Fit attachments and the \"horrible hack\" that this PR removes, one potential refactoring we must evaluate is to provide components vetoing power on the modification of an SED. We had a number of issues in v2.1 that had to do with the fact that specview didn't like (and didn't do much about) users modifying SEDs. While the previous implementation was not perfect, it did indicate that the current Iris framework is lacking the ability of components to veto changes in SED, and to react to such changes. This is a design task left for the future."},"comment":{"url":"https://api.github.com/repos/ChandraCXC/iris/issues/comments/152046244","html_url":"https://github.com/ChandraCXC/iris/pull/168#issuecomment-152046244","issue_url":"https://api.github.com/repos/ChandraCXC/iris/issues/168","id":152046244,"user":{"login":"eholum","id":7966037,"avatar_url":"https://avatars.githubusercontent.com/u/7966037?v=3","gravatar_id":"","url":"https://api.github.com/users/eholum","html_url":"https://github.com/eholum","followers_url":"https://api.github.com/users/eholum/followers","following_url":"https://api.github.com/users/eholum/following{/other_user}","gists_url":"https://api.github.com/users/eholum/gists{/gist_id}","starred_url":"https://api.github.com/users/eholum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eholum/subscriptions","organizations_url":"https://api.github.com/users/eholum/orgs","repos_url":"https://api.github.com/users/eholum/repos","events_url":"https://api.github.com/users/eholum/events{/privacy}","received_events_url":"https://api.github.com/users/eholum/received_events","type":"User","site_admin":false},"created_at":"2015-10-29T01:18:09Z","updated_at":"2015-10-29T01:18:09Z","body":"It looks good to me. I like have the dummy implementation available to know what we're going to have to be dealing with.\r\n\r\nRather than static singleton instance, I agree with the above and think it would be better to have whatever units functionality we need contained in a class variable of the core iris application (putting it in the workspace sounds good since we're hopefully going to be making changes to the SED manager). My impression is that units conversion is a crucial part of Iris's public API  - and it seems like most callers/components will want to have access to it, and if 3rd party users have some special units package they should be able to pass that in in place of whatever we have as the default. The interface looks simple enough that if somebody did want to provide their own implementation (for some odd reason) that Iris could offer that.\r\n\r\nDo we have an idea of how hard it would be to rip SPECView unit conversion classes and just put them in a temporary location in iris-common? Otherwise if it's just the smoke tests/other tests that need it I'm fine leaving the dependency in until we complete a \"implement units conversion code\" task done. We just need to be sure to keep a task in place to remove it when we have our own package.\r\n\r\nAnd let's talk more about the need for \"veto\" powers offline. That does make for some interesting complications with the MVC or IOC architecture that's currently in place. We could probably just come up with some kind of state variable in the application that determines that and can be set by individual components when needed - rather than having to get approval from a component when firing off a change notification."}}