{"action":"created","issue":{"url":"https://api.github.com/repos/QuantStack/xtensor/issues/119","repository_url":"https://api.github.com/repos/QuantStack/xtensor","labels_url":"https://api.github.com/repos/QuantStack/xtensor/issues/119/labels{/name}","comments_url":"https://api.github.com/repos/QuantStack/xtensor/issues/119/comments","events_url":"https://api.github.com/repos/QuantStack/xtensor/issues/119/events","html_url":"https://github.com/QuantStack/xtensor/pull/119","id":205974811,"number":119,"title":"WIP xdynview","user":{"login":"wolfv","id":885054,"avatar_url":"https://avatars3.githubusercontent.com/u/885054?v=3","gravatar_id":"","url":"https://api.github.com/users/wolfv","html_url":"https://github.com/wolfv","followers_url":"https://api.github.com/users/wolfv/followers","following_url":"https://api.github.com/users/wolfv/following{/other_user}","gists_url":"https://api.github.com/users/wolfv/gists{/gist_id}","starred_url":"https://api.github.com/users/wolfv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wolfv/subscriptions","organizations_url":"https://api.github.com/users/wolfv/orgs","repos_url":"https://api.github.com/users/wolfv/repos","events_url":"https://api.github.com/users/wolfv/events{/privacy}","received_events_url":"https://api.github.com/users/wolfv/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":17,"created_at":"2017-02-07T18:30:22Z","updated_at":"2017-03-14T16:16:43Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/QuantStack/xtensor/pulls/119","html_url":"https://github.com/QuantStack/xtensor/pull/119","diff_url":"https://github.com/QuantStack/xtensor/pull/119.diff","patch_url":"https://github.com/QuantStack/xtensor/pull/119.patch"},"body":"This PR implements a dynamic view type, which is represented by a std::vector of variants.\r\n\r\n(This is more because I want to see if it builds on the other platforms)."},"comment":{"url":"https://api.github.com/repos/QuantStack/xtensor/issues/comments/286473829","html_url":"https://github.com/QuantStack/xtensor/pull/119#issuecomment-286473829","issue_url":"https://api.github.com/repos/QuantStack/xtensor/issues/119","id":286473829,"user":{"login":"JohanMabille","id":6754742,"avatar_url":"https://avatars3.githubusercontent.com/u/6754742?v=3","gravatar_id":"","url":"https://api.github.com/users/JohanMabille","html_url":"https://github.com/JohanMabille","followers_url":"https://api.github.com/users/JohanMabille/followers","following_url":"https://api.github.com/users/JohanMabille/following{/other_user}","gists_url":"https://api.github.com/users/JohanMabille/gists{/gist_id}","starred_url":"https://api.github.com/users/JohanMabille/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JohanMabille/subscriptions","organizations_url":"https://api.github.com/users/JohanMabille/orgs","repos_url":"https://api.github.com/users/JohanMabille/repos","events_url":"https://api.github.com/users/JohanMabille/events{/privacy}","received_events_url":"https://api.github.com/users/JohanMabille/received_events","type":"User","site_admin":false},"created_at":"2017-03-14T16:16:43Z","updated_at":"2017-03-14T16:16:43Z","body":"Ok after a more detailed review / thinking of this, I think the variant approach would have the following drawbacks:\r\n\r\n- listing of ALL the possible types, so each time we add a new type of slice, we modify the variant declaration and we add a visitor method.\r\n- this would require an implicit conversion for each slice type (from XXX<int_type1> to XXX<int_type2>) if we don't want to enumerate every possible instantiated slice type (and that would be huge) while being able to store every of them.\r\n\r\nSo let's keep the any approach for the moment, we could reconsider (or not) the variant approach if we experience performance issue (because no dynamic allocation occurs with variants).\r\n\r\nNow a major concern for me is this switch on slice type (bunch of if - conditions) which makes the code hard to read and hard to maintain. A better solution would be to register a mapping ``type_info`` - structures that provide interfaces to build the slices / retrieve the index / etc. More precisely we would have to store the hashcode of the ``type_info`` or a ``type_index``, I don't know which one is better. This solution would also facilitate building the view from either ``int`` or ``std::size_t``.\r\n\r\nHowever this requires a lot of more work, you've already done a lot and you may need this feature to be merged quickly. So let me know which option you prefer, either I can merge it now and we can refactor later or you can start the refactoring and I merge it once you're done. If you choose the second option, I can give you more details if you want, I have some ideas :)"}}
{"actor":{"display_login":"JohanMabille"}}