{"action":"created","issue":{"url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1603","labels_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1603/labels{/name}","comments_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1603/comments","events_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1603/events","html_url":"https://github.com/ImageEngine/gaffer/pull/1603","id":123170469,"number":1603,"title":"ImageAlgo : Add additional channelExists() overload which takes string vector","user":{"login":"HughMacdonald","id":625512,"avatar_url":"https://avatars.githubusercontent.com/u/625512?v=3","gravatar_id":"","url":"https://api.github.com/users/HughMacdonald","html_url":"https://github.com/HughMacdonald","followers_url":"https://api.github.com/users/HughMacdonald/followers","following_url":"https://api.github.com/users/HughMacdonald/following{/other_user}","gists_url":"https://api.github.com/users/HughMacdonald/gists{/gist_id}","starred_url":"https://api.github.com/users/HughMacdonald/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/HughMacdonald/subscriptions","organizations_url":"https://api.github.com/users/HughMacdonald/orgs","repos_url":"https://api.github.com/users/HughMacdonald/repos","events_url":"https://api.github.com/users/HughMacdonald/events{/privacy}","received_events_url":"https://api.github.com/users/HughMacdonald/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/ImageEngine/gaffer/labels/pr-revisionRequired","name":"pr-revisionRequired","color":"FFAAAA"}],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-12-20T16:33:12Z","updated_at":"2015-12-22T15:32:00Z","closed_at":"2015-12-22T15:31:46Z","pull_request":{"url":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1603","html_url":"https://github.com/ImageEngine/gaffer/pull/1603","diff_url":"https://github.com/ImageEngine/gaffer/pull/1603.diff","patch_url":"https://github.com/ImageEngine/gaffer/pull/1603.patch"},"body":"This PR adds a second channelExists() method to ImageAlgo to allow for a `vector<string>` to be passed straight in, so that if it's called twice in a row, it doesn't have to request channelNamesPlug() to give its value.\r\n\r\nThere are a couple of issues I had/have with this:\r\n\r\nIn ImageAlgoBinding, I couldn't figure out how to specify the overloaded method directly there without going through a helper method. I found lots of examples of how to do it with a member method, but not when the method was stand-alone.\r\n\r\nI tried something like:\r\n\r\n    def( \"channelExists\", bool ()( const ImagePlug *image, const std::string &channelName ) const )&GafferImage::channelExists );\r\n    def( \"channelExists\", bool ()( const std::vector<std::string> &channelNames, const std::string &channelName ) const )&GafferImage::channelExists );\r\n\r\nBut it was what went in the brackets after `bool` that I couldn't get. For a member method, it would be `ClassName::*`, but I couldn't figure out what to do here.\r\n\r\nThe second thing was having it implicitly convert StringVectorData in Python to vector<string> in C++. The code as it currently stands errors on the test, with:\r\n\r\n    ArgumentError: Python argument types in\r\n        GafferImage._GafferImage.channelExists(StringVectorData, str)\r\n    did not match C++ signature:\r\n        channelExists(std::vector<std::string, std::allocator<std::string> >, std::string)\r\n        channelExists(GafferImage::ImagePlug const*, std::string)\r\n\r\nHowever, looking at PathMatcherBinding, something very similar is being done and it works.\r\n\r\nhttps://github.com/ImageEngine/gaffer/blob/master/src/GafferSceneBindings/PathMatcherBinding.cpp#L127\r\n\r\n    .def( \"match\", (unsigned (PathMatcher ::*)( const std::vector<IECore::InternedString> & ) const)&PathMatcher::match )\r\n\r\nDefines the binding as linking to a method that accepts a vector. However, here:\r\n\r\nhttps://github.com/ImageEngine/gaffer/blob/master/python/GafferSceneTest/PathMatcherTest.py#L547\r\n\r\n    self.assertEqual(\r\n        m.match( IECore.InternedStringVectorData( path[1:].split( \"/\" ) ) ),\r\n        int( result ),\r\n    )\r\n\r\nmatch() is called with an InternedStringVectorData.\r\n\r\nThe only difference in my case is that I'm using a StringVectorData rather than an InternedStringVectorData, but I would have expected the process to be the same - that the conversion is being done from `[type]VectorData` to `vector<[type]>`\r\n\r\n\r\nOn a side note, I hope it's okay for me to be doing PRs that are \"Here's a PR - it's not quite finished, but here are my questions about it\"? Nobody's complained (to me, at least), but I'm aware that my PRs tend to end up with a lot more conversation than others..."},"comment":{"url":"https://api.github.com/repos/ImageEngine/gaffer/issues/comments/166648045","html_url":"https://github.com/ImageEngine/gaffer/pull/1603#issuecomment-166648045","issue_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1603","id":166648045,"user":{"login":"johnhaddon","id":1133871,"avatar_url":"https://avatars.githubusercontent.com/u/1133871?v=3","gravatar_id":"","url":"https://api.github.com/users/johnhaddon","html_url":"https://github.com/johnhaddon","followers_url":"https://api.github.com/users/johnhaddon/followers","following_url":"https://api.github.com/users/johnhaddon/following{/other_user}","gists_url":"https://api.github.com/users/johnhaddon/gists{/gist_id}","starred_url":"https://api.github.com/users/johnhaddon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johnhaddon/subscriptions","organizations_url":"https://api.github.com/users/johnhaddon/orgs","repos_url":"https://api.github.com/users/johnhaddon/repos","events_url":"https://api.github.com/users/johnhaddon/events{/privacy}","received_events_url":"https://api.github.com/users/johnhaddon/received_events","type":"User","site_admin":false},"created_at":"2015-12-22T15:32:00Z","updated_at":"2015-12-22T15:32:00Z","body":"Thanks Hugh."}}