{"action":"created","issue":{"url":"https://api.github.com/repos/allenai/allennlp/issues/1232","repository_url":"https://api.github.com/repos/allenai/allennlp","labels_url":"https://api.github.com/repos/allenai/allennlp/issues/1232/labels{/name}","comments_url":"https://api.github.com/repos/allenai/allennlp/issues/1232/comments","events_url":"https://api.github.com/repos/allenai/allennlp/issues/1232/events","html_url":"https://github.com/allenai/allennlp/pull/1232","id":323841004,"number":1232,"title":"Fix test-install by moving tests into module","user":{"login":"nelson-liu","id":7272031,"avatar_url":"https://avatars3.githubusercontent.com/u/7272031?v=4","gravatar_id":"","url":"https://api.github.com/users/nelson-liu","html_url":"https://github.com/nelson-liu","followers_url":"https://api.github.com/users/nelson-liu/followers","following_url":"https://api.github.com/users/nelson-liu/following{/other_user}","gists_url":"https://api.github.com/users/nelson-liu/gists{/gist_id}","starred_url":"https://api.github.com/users/nelson-liu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nelson-liu/subscriptions","organizations_url":"https://api.github.com/users/nelson-liu/orgs","repos_url":"https://api.github.com/users/nelson-liu/repos","events_url":"https://api.github.com/users/nelson-liu/events{/privacy}","received_events_url":"https://api.github.com/users/nelson-liu/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"schmmd","id":954798,"avatar_url":"https://avatars3.githubusercontent.com/u/954798?v=4","gravatar_id":"","url":"https://api.github.com/users/schmmd","html_url":"https://github.com/schmmd","followers_url":"https://api.github.com/users/schmmd/followers","following_url":"https://api.github.com/users/schmmd/following{/other_user}","gists_url":"https://api.github.com/users/schmmd/gists{/gist_id}","starred_url":"https://api.github.com/users/schmmd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/schmmd/subscriptions","organizations_url":"https://api.github.com/users/schmmd/orgs","repos_url":"https://api.github.com/users/schmmd/repos","events_url":"https://api.github.com/users/schmmd/events{/privacy}","received_events_url":"https://api.github.com/users/schmmd/received_events","type":"User","site_admin":false},"assignees":[{"login":"schmmd","id":954798,"avatar_url":"https://avatars3.githubusercontent.com/u/954798?v=4","gravatar_id":"","url":"https://api.github.com/users/schmmd","html_url":"https://github.com/schmmd","followers_url":"https://api.github.com/users/schmmd/followers","following_url":"https://api.github.com/users/schmmd/following{/other_user}","gists_url":"https://api.github.com/users/schmmd/gists{/gist_id}","starred_url":"https://api.github.com/users/schmmd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/schmmd/subscriptions","organizations_url":"https://api.github.com/users/schmmd/orgs","repos_url":"https://api.github.com/users/schmmd/repos","events_url":"https://api.github.com/users/schmmd/events{/privacy}","received_events_url":"https://api.github.com/users/schmmd/received_events","type":"User","site_admin":false}],"milestone":null,"comments":12,"created_at":"2018-05-17T01:26:50Z","updated_at":"2018-05-25T16:20:36Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/allenai/allennlp/pulls/1232","html_url":"https://github.com/allenai/allennlp/pull/1232","diff_url":"https://github.com/allenai/allennlp/pull/1232.diff","patch_url":"https://github.com/allenai/allennlp/pull/1232.patch"},"body":"Goals of this PR:\r\n1. [x] 1. Prevent tests from being installed as a module in `python setup.py` . Currently, pip copies them to `site-packages`, which is definitely not intended behavior.\r\n2. [x] 2. installation method-invariant way to load and run tests, especially those that depend on files outside of the module (e.g. `scripts` or `tutorials`). The issue is that files can be in different places depending on if allennlp is installed with `pip` or `setup.py install`.\r\n\r\nFor 2: an easy solution might be to, when running the `test-install` command, simply download and cache a copy of the allennlp source for the appropriate version at `~/.allennlp` . Note that this wouldn't work for non-release versions, but I think that's a good thing --- if you're working on master or developing features, you shouldn't be using `test-install` anyway.\r\n\r\nThoughts? I'm not sure how i feel about the above solution, so feedback would be great before i write the code.\r\n\r\n----------------------------------------------------------------------------------\r\n\r\n*Edit*: another solution is to just move the tests into the `allennlp` module (this is a fairly standard way of structuring projects anyway). The downside to this is that it still doesn't enable using `allennlp test-install` to run tests in `tutorials`, as well as the tests that depend on files outside the module (e.g. SRL test that compares output of evaluator with a perl script). But maybe we shouldn't be running those tests anyway in `test-install`? or we can move the perl script into the tests and then just not run the notebook tests, since we'll be rewriting them anyway.\r\n\r\nThis solution is rather time-intensive, though, since it involves fixing all of the paths that are hardcoded into the tests. Should not be too hard to write a script to go through and change the paths, minimizing human labor to just a check up..."},"comment":{"url":"https://api.github.com/repos/allenai/allennlp/issues/comments/392109352","html_url":"https://github.com/allenai/allennlp/pull/1232#issuecomment-392109352","issue_url":"https://api.github.com/repos/allenai/allennlp/issues/1232","id":392109352,"user":{"login":"DeNeutoy","id":16001974,"avatar_url":"https://avatars1.githubusercontent.com/u/16001974?v=4","gravatar_id":"","url":"https://api.github.com/users/DeNeutoy","html_url":"https://github.com/DeNeutoy","followers_url":"https://api.github.com/users/DeNeutoy/followers","following_url":"https://api.github.com/users/DeNeutoy/following{/other_user}","gists_url":"https://api.github.com/users/DeNeutoy/gists{/gist_id}","starred_url":"https://api.github.com/users/DeNeutoy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DeNeutoy/subscriptions","organizations_url":"https://api.github.com/users/DeNeutoy/orgs","repos_url":"https://api.github.com/users/DeNeutoy/repos","events_url":"https://api.github.com/users/DeNeutoy/events{/privacy}","received_events_url":"https://api.github.com/users/DeNeutoy/received_events","type":"User","site_admin":false},"created_at":"2018-05-25T16:20:35Z","updated_at":"2018-05-25T16:20:35Z","author_association":"CONTRIBUTOR","body":"I was semi-serious about not liking the `Path` thing - I think it will just cause us to have loads of `str(path)` things everywhere and doesn't really add much. I'd kinda prefer it if this PR switched back to using string paths.... @joelgrus lets argue"}}
{"actor":{"display_login":"DeNeutoy"}}