{"action":"created","comment":{"url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/comments/219571418","pull_request_review_id":157781408,"id":219571418,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIxOTU3MTQxOA==","diff_hunk":"@@ -650,105 +771,40 @@ Following :ref:`this rule <style-guide-pybind11-declare-template-wrappers>` we d\n       - ``D`` for ``double`` and\n       - ``U`` for ``unsigned int``.\n \n-Cross module imports\n-^^^^^^^^^^^^^^^^^^^^\n-\n-The import statement:\n-\n-.. code-block:: cpp\n-\n-    py::module::import(\"exampleTwo\");\n-\n-in the previous example is present because ``ExampleThree`` depends on ``ExampleBase`` which is defined in a different module (i.e. ``exampleTwo``).\n-\n-Thus, if you forget to add the import statement, the type ``ExampleBase`` is unknown if ``exampleThree`` happens to be imported before ``exampleTwo``.\n-\n-Adding these import statements in the C++ wrapper, rather than relying on import order in ``__init__`` in Python, follows :ref:`our rule on import <style-guide-pybind11-import>`.\n \n-Finished wrappers (C++ part)\n-^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n+Cross-module dependencies\n+^^^^^^^^^^^^^^^^^^^^^^^^^\n \n-The end results for the C++ part of the wrappers (see next for the Python part) are ``exampleTwo.cc``:\n+All of the depenxencies in the example classes we've defined here are within the same compiled module, and hence we've been able to rely on ``WrapperCollection`` and its callback system to ensure the wrappers are defined in an order that works.\n+The only exception is inheritance: ``ExampleTwo`` and ``ExampleThree`` both inherit from ``ExampleBase``, and that means it's critical that the ``wrapType`` call (or more precisely, the ``py::class_`` instantiation) for ``ExampleBase`` appears before that of either of its derived classes.\n \n-.. code-block:: cpp\n-\n-    #include \"pybind11/pybind11.h\"\n-\n-    #include \"lsst/TMPL/ExampleTwo.h\"\n+When dependencies extend beyond module boundaries, we need to import the modules that provide the classes we're using.\n+While it's possible to do that directly with ``py::module::import`` calls, it's more readable and less error-prone to use the ``addInheritanceDependency`` and ``addSignatureDependency`` methods of ``WrapperCollection``.\n+As its name suggests, the former is needed when you want to inherit from a base class defined in another module; this must be called before the ``py::class_`` instantiation of the derived class.\n+``addSignatureDependency`` declares that the external type is used only in function or method signatures.\n+Regardless of when it is called, these dependencies will be imported after all local types are declared and before any definition callbacks are run.\n \n-    namespace py = pybind11;\n-    using namespace pybind11::literals;\n-\n-    namespace lsst {\n-    namespace tmpl {\n+Circular *inheritance* dependencies are impossible in both C++ and Python, but circular signatures dependencies are relatively common in C++ (via forward declarations) and quite possible in Python (via function-level imports and duck-typing).\n+In pybind11 wrappers, circular signature dependencies are a problem.\n+Using ``WrapperCollection`` solves that problem within a single module (in which all wrappers are added to the same ``WrapperCollection`` instance), and it takes steps to solve across multiple modules, by executing signature-dependency imports between type definitions and function definitions.\n+But this only solves cross-module circular dependencies to the extent that a circular Python imports result let the second importer see the types in the partially-defined first module.\n+It's not clearly that something guaranteed by the CPython interpreter (let alone the Python language), so it's much safer too just make sure any types with a circular dependency are handled by the same ``WrapperCollection`` instance and defined in the same module.","path":"pybind11/how-to.rst","position":721,"original_position":721,"commit_id":"77516e0ec9d000b9173d48035ba5a6422da5eda0","original_commit_id":"77516e0ec9d000b9173d48035ba5a6422da5eda0","user":{"login":"ktlim","id":1054393,"node_id":"MDQ6VXNlcjEwNTQzOTM=","avatar_url":"https://avatars3.githubusercontent.com/u/1054393?v=4","gravatar_id":"","url":"https://api.github.com/users/ktlim","html_url":"https://github.com/ktlim","followers_url":"https://api.github.com/users/ktlim/followers","following_url":"https://api.github.com/users/ktlim/following{/other_user}","gists_url":"https://api.github.com/users/ktlim/gists{/gist_id}","starred_url":"https://api.github.com/users/ktlim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ktlim/subscriptions","organizations_url":"https://api.github.com/users/ktlim/orgs","repos_url":"https://api.github.com/users/ktlim/repos","events_url":"https://api.github.com/users/ktlim/events{/privacy}","received_events_url":"https://api.github.com/users/ktlim/received_events","type":"User","site_admin":false},"body":"Remove \"that\".  \"too\" -> \"to\".  Is it always possible to have circularly-dependent types defined in the same module?","created_at":"2018-09-21T17:27:55Z","updated_at":"2018-09-21T17:40:19Z","html_url":"https://github.com/lsst-dm/dm_dev_guide/pull/262#discussion_r219571418","pull_request_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/262","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/comments/219571418"},"html":{"href":"https://github.com/lsst-dm/dm_dev_guide/pull/262#discussion_r219571418"},"pull_request":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/262"}}},"pull_request":{"url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/262","id":217145124,"node_id":"MDExOlB1bGxSZXF1ZXN0MjE3MTQ1MTI0","html_url":"https://github.com/lsst-dm/dm_dev_guide/pull/262","diff_url":"https://github.com/lsst-dm/dm_dev_guide/pull/262.diff","patch_url":"https://github.com/lsst-dm/dm_dev_guide/pull/262.patch","issue_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues/262","number":262,"state":"open","locked":false,"title":"DM-10384: rewrite pybind11 how-to and style guide","user":{"login":"TallJimbo","id":1330720,"node_id":"MDQ6VXNlcjEzMzA3MjA=","avatar_url":"https://avatars0.githubusercontent.com/u/1330720?v=4","gravatar_id":"","url":"https://api.github.com/users/TallJimbo","html_url":"https://github.com/TallJimbo","followers_url":"https://api.github.com/users/TallJimbo/followers","following_url":"https://api.github.com/users/TallJimbo/following{/other_user}","gists_url":"https://api.github.com/users/TallJimbo/gists{/gist_id}","starred_url":"https://api.github.com/users/TallJimbo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TallJimbo/subscriptions","organizations_url":"https://api.github.com/users/TallJimbo/orgs","repos_url":"https://api.github.com/users/TallJimbo/repos","events_url":"https://api.github.com/users/TallJimbo/events{/privacy}","received_events_url":"https://api.github.com/users/TallJimbo/received_events","type":"User","site_admin":false},"body":"","created_at":"2018-09-21T00:56:08Z","updated_at":"2018-09-21T17:40:18Z","closed_at":null,"merged_at":null,"merge_commit_sha":"5f3601bbc4acb23957dc238337336cb6a9f68fe0","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/262/commits","review_comments_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/262/comments","review_comment_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/comments{/number}","comments_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues/262/comments","statuses_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/statuses/77516e0ec9d000b9173d48035ba5a6422da5eda0","head":{"label":"lsst-dm:tickets/DM-10384","ref":"tickets/DM-10384","sha":"77516e0ec9d000b9173d48035ba5a6422da5eda0","user":{"login":"lsst-dm","id":10968870,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEwOTY4ODcw","avatar_url":"https://avatars1.githubusercontent.com/u/10968870?v=4","gravatar_id":"","url":"https://api.github.com/users/lsst-dm","html_url":"https://github.com/lsst-dm","followers_url":"https://api.github.com/users/lsst-dm/followers","following_url":"https://api.github.com/users/lsst-dm/following{/other_user}","gists_url":"https://api.github.com/users/lsst-dm/gists{/gist_id}","starred_url":"https://api.github.com/users/lsst-dm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lsst-dm/subscriptions","organizations_url":"https://api.github.com/users/lsst-dm/orgs","repos_url":"https://api.github.com/users/lsst-dm/repos","events_url":"https://api.github.com/users/lsst-dm/events{/privacy}","received_events_url":"https://api.github.com/users/lsst-dm/received_events","type":"Organization","site_admin":false},"repo":{"id":40620718,"node_id":"MDEwOlJlcG9zaXRvcnk0MDYyMDcxOA==","name":"dm_dev_guide","full_name":"lsst-dm/dm_dev_guide","private":false,"owner":{"login":"lsst-dm","id":10968870,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEwOTY4ODcw","avatar_url":"https://avatars1.githubusercontent.com/u/10968870?v=4","gravatar_id":"","url":"https://api.github.com/users/lsst-dm","html_url":"https://github.com/lsst-dm","followers_url":"https://api.github.com/users/lsst-dm/followers","following_url":"https://api.github.com/users/lsst-dm/following{/other_user}","gists_url":"https://api.github.com/users/lsst-dm/gists{/gist_id}","starred_url":"https://api.github.com/users/lsst-dm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lsst-dm/subscriptions","organizations_url":"https://api.github.com/users/lsst-dm/orgs","repos_url":"https://api.github.com/users/lsst-dm/repos","events_url":"https://api.github.com/users/lsst-dm/events{/privacy}","received_events_url":"https://api.github.com/users/lsst-dm/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/lsst-dm/dm_dev_guide","description":"LSST Data Management Developer Guide","fork":false,"url":"https://api.github.com/repos/lsst-dm/dm_dev_guide","forks_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/forks","keys_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/keys{/key_id}","collaborators_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/teams","hooks_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/hooks","issue_events_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues/events{/number}","events_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/events","assignees_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/assignees{/user}","branches_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/branches{/branch}","tags_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/tags","blobs_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/refs{/sha}","trees_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/trees{/sha}","statuses_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/statuses/{sha}","languages_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/languages","stargazers_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/stargazers","contributors_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/contributors","subscribers_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/subscribers","subscription_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/subscription","commits_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/commits{/sha}","git_commits_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/commits{/sha}","comments_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/comments{/number}","issue_comment_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues/comments{/number}","contents_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/contents/{+path}","compare_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/compare/{base}...{head}","merges_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/merges","archive_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/downloads","issues_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues{/number}","pulls_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls{/number}","milestones_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/milestones{/number}","notifications_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/labels{/name}","releases_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/releases{/id}","deployments_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/deployments","created_at":"2015-08-12T19:43:19Z","updated_at":"2018-09-19T21:08:51Z","pushed_at":"2018-09-21T00:56:09Z","git_url":"git://github.com/lsst-dm/dm_dev_guide.git","ssh_url":"git@github.com:lsst-dm/dm_dev_guide.git","clone_url":"https://github.com/lsst-dm/dm_dev_guide.git","svn_url":"https://github.com/lsst-dm/dm_dev_guide","homepage":"https://developer.lsst.io","size":8556,"stargazers_count":5,"watchers_count":5,"language":"Python","has_issues":true,"has_projects":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":7,"mirror_url":null,"archived":false,"open_issues_count":6,"license":{"key":"cc-by-4.0","name":"Creative Commons Attribution 4.0","spdx_id":"CC-BY-4.0","url":"https://api.github.com/licenses/cc-by-4.0","node_id":"MDc6TGljZW5zZTI1"},"forks":7,"open_issues":6,"watchers":5,"default_branch":"master"}},"base":{"label":"lsst-dm:master","ref":"master","sha":"264171242cc67e2d0ecfbbc30fbbfcd8b318ceb1","user":{"login":"lsst-dm","id":10968870,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEwOTY4ODcw","avatar_url":"https://avatars1.githubusercontent.com/u/10968870?v=4","gravatar_id":"","url":"https://api.github.com/users/lsst-dm","html_url":"https://github.com/lsst-dm","followers_url":"https://api.github.com/users/lsst-dm/followers","following_url":"https://api.github.com/users/lsst-dm/following{/other_user}","gists_url":"https://api.github.com/users/lsst-dm/gists{/gist_id}","starred_url":"https://api.github.com/users/lsst-dm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lsst-dm/subscriptions","organizations_url":"https://api.github.com/users/lsst-dm/orgs","repos_url":"https://api.github.com/users/lsst-dm/repos","events_url":"https://api.github.com/users/lsst-dm/events{/privacy}","received_events_url":"https://api.github.com/users/lsst-dm/received_events","type":"Organization","site_admin":false},"repo":{"id":40620718,"node_id":"MDEwOlJlcG9zaXRvcnk0MDYyMDcxOA==","name":"dm_dev_guide","full_name":"lsst-dm/dm_dev_guide","private":false,"owner":{"login":"lsst-dm","id":10968870,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEwOTY4ODcw","avatar_url":"https://avatars1.githubusercontent.com/u/10968870?v=4","gravatar_id":"","url":"https://api.github.com/users/lsst-dm","html_url":"https://github.com/lsst-dm","followers_url":"https://api.github.com/users/lsst-dm/followers","following_url":"https://api.github.com/users/lsst-dm/following{/other_user}","gists_url":"https://api.github.com/users/lsst-dm/gists{/gist_id}","starred_url":"https://api.github.com/users/lsst-dm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lsst-dm/subscriptions","organizations_url":"https://api.github.com/users/lsst-dm/orgs","repos_url":"https://api.github.com/users/lsst-dm/repos","events_url":"https://api.github.com/users/lsst-dm/events{/privacy}","received_events_url":"https://api.github.com/users/lsst-dm/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/lsst-dm/dm_dev_guide","description":"LSST Data Management Developer Guide","fork":false,"url":"https://api.github.com/repos/lsst-dm/dm_dev_guide","forks_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/forks","keys_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/keys{/key_id}","collaborators_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/teams","hooks_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/hooks","issue_events_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues/events{/number}","events_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/events","assignees_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/assignees{/user}","branches_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/branches{/branch}","tags_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/tags","blobs_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/refs{/sha}","trees_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/trees{/sha}","statuses_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/statuses/{sha}","languages_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/languages","stargazers_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/stargazers","contributors_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/contributors","subscribers_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/subscribers","subscription_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/subscription","commits_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/commits{/sha}","git_commits_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/git/commits{/sha}","comments_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/comments{/number}","issue_comment_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues/comments{/number}","contents_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/contents/{+path}","compare_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/compare/{base}...{head}","merges_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/merges","archive_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/downloads","issues_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues{/number}","pulls_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls{/number}","milestones_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/milestones{/number}","notifications_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/labels{/name}","releases_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/releases{/id}","deployments_url":"https://api.github.com/repos/lsst-dm/dm_dev_guide/deployments","created_at":"2015-08-12T19:43:19Z","updated_at":"2018-09-19T21:08:51Z","pushed_at":"2018-09-21T00:56:09Z","git_url":"git://github.com/lsst-dm/dm_dev_guide.git","ssh_url":"git@github.com:lsst-dm/dm_dev_guide.git","clone_url":"https://github.com/lsst-dm/dm_dev_guide.git","svn_url":"https://github.com/lsst-dm/dm_dev_guide","homepage":"https://developer.lsst.io","size":8556,"stargazers_count":5,"watchers_count":5,"language":"Python","has_issues":true,"has_projects":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":7,"mirror_url":null,"archived":false,"open_issues_count":6,"license":{"key":"cc-by-4.0","name":"Creative Commons Attribution 4.0","spdx_id":"CC-BY-4.0","url":"https://api.github.com/licenses/cc-by-4.0","node_id":"MDc6TGljZW5zZTI1"},"forks":7,"open_issues":6,"watchers":5,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/262"},"html":{"href":"https://github.com/lsst-dm/dm_dev_guide/pull/262"},"issue":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues/262"},"comments":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/issues/262/comments"},"review_comments":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/262/comments"},"review_comment":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/pulls/262/commits"},"statuses":{"href":"https://api.github.com/repos/lsst-dm/dm_dev_guide/statuses/77516e0ec9d000b9173d48035ba5a6422da5eda0"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"ktlim"}}