{"action":"created","comment":{"url":"https://api.github.com/repos/percious/crunch-fitness/pulls/comments/130973164","pull_request_review_id":53906532,"id":130973164,"diff_hunk":"@@ -5,47 +5,116 @@\n \"\"\"\n \n import os\n-from cr.db.loader import load_data, load_dataset\n+import pytest\n+import matplotlib.pyplot as plt\n+\n+from cr.db.loader import Loader\n from cr.db.store import global_settings as settings\n from cr.db.store import connect\n+from textwrap import wrap\n \n settings.update({\"url\": \"mongodb://localhost:27017/test_crunch_fitness\"})\n db = connect(settings)\n \n _here = os.path.dirname(__file__)\n \n+RAW_COLUMN_DATA = ['CAT-1', 'CAT-2', 'CAT-1', 'CAT-3']\n+\n+\n def test_loader():\n     \"\"\"\n-    Is this the most efficient way that we could load users?  What if the file had 1m users?\n-    How would/could you benchmark this?\n+    Is this the most efficient way that we could load users? What if the file\n+    had 1m users? How would/could you benchmark this?\n+    Slobodan's Answer:\n+    1. Not the most efficient way because of 2 reasons:\n+        - Entire file is loaded into memory for JSON loading\n+        - Inserting is done for each object separately, which is not optimal\n+          because it talks to MongoDB each time\n+    2. We can employ two strategies for optimization:\n+        - Using iterative way of parsig JSON file (we save RAM)\n+        - Using single insert (or insert_many) to talk to Mongo\n+    3. For benchmark I created an actual file of 1M users and tested how the\n+        loading behaved with Process Monitor. For a more formal benchmarking\n+        I'd probalby use some of the python frameworks for bencharking\n+        (time and memory)\n     \"\"\"\n \n-    load_data(_here + '/data/users.json', settings=settings, clear=True)\n+    Loader.load_data(_here + '/data/users.json', settings=settings, clear=True,\n+                     optimize_for='speed')\n     assert db.users.count() == 10, db.users.count()\n \n-def test_load_dataset():\n \n-    #data_filename = _here + '/data/Stack-Overflow-Developer-Survey-2017.csv.zip'\n+def test_load_small_dataset_equality():\n+    csv_filename = _here + '/data/S-O-small.csv'\n+    ds_id = Loader.load_dataset(csv_filename, db, modify_to='to_string',\n+                                max_categories=2)\n+    expected_headers = [\n+        'PronounceGIF', 'SalaryAdjusted', 'Combined Gender',\n+        'Combined Gender', 'Combined Gender',\n+    ]\n+    expected_columns = [\n+        '1,0,0',\n+        ['60000', '70000', '113750'],\n+        '1,0,0',\n+        '0,0,1',\n+        '0,1,0',\n+    ]\n+    actual_headers = db.datasets.find({'_id': ds_id})[0]['headers']\n+    actual_columns = db.datasets.find({'_id': ds_id})[0]['columns']\n+\n+    assert actual_headers == expected_headers\n+    assert actual_columns == expected_columns\n \n-    #if not os.path.exists(csv_filename):\n-    #    with zipfile.ZipFile(data_filename, 'r') as zipref:\n-    #        zipref.extractall(_here + '/data')\n \n+def test_load_dataset():\n     csv_filename = _here + '/data/S-O-1k.csv'\n+    ds_id = Loader.load_dataset(csv_filename, db, modify_to='to_string',\n+                                max_categories=50)\n \n-    ds_id = load_dataset(csv_filename, db)\n-\n-    columns = db.datasets.find({'_id': ds_id})[0]['columns']\n+    dataset = db.datasets.find({'_id': ds_id})[0]\n \n     # can you make a meaningful assertion?\n+    # Slobodan's answer:\n+    # Since the dataset is huge, it doesn't make much sense\n+    # to assert the equality of raw data. We could employ two\n+    # strategies:\n+    # 1. Create a small dataset with all the relevant cases\n+    #   (categories, quantitative and missing data), and\n+    #   compare a result, as in 'test_load_small_dataset_equality'\n+    # 2. Compare a result of a certain processing of the dataset\n+    #   for which the actual dataset has to be loaded correctly\n+    #   such as in the following assertion, which compares if the\n+    #   number of rows with the answer \"Some other way\" is equal to 13\n+    #   which is the knowledge obtained from CSV file\n+    answer_encoding = dataset['encoding'][0]['Some other way']\n+    expected_result = 13  # 13 answers \"Some other way\"\n+    actual_result = len([\n+        ans for ans in dataset['columns'][0].split(',')\n+        if int(ans) == answer_encoding\n+    ])\n+    assert actual_result == expected_result\n \n     # the columns aren't terribly useful.  Modify load_dataset to load common responses as integers so we can\n     #   do data manipulation.  For instance, you could change the gender column to male = 0 female = 1 (or something)\n \n     # you _should_ be able to save S-O-10k if you convert booleans to boolean and use integers for categories.\n+    # Slobodan's answer:\n+    # Even with converting everything to int, bool, and None, I was only able\n+    # to get the BSON document size from about 66M down to about 30M, and the\n+    # max size set in MongoDB is about 16M. So I resorted to storing the data\n+    # as joined strings, which worked just fine.","path":"cr-db/tests/test_db.py","position":113,"original_position":113,"commit_id":"1b2b82e3dbeb1b53ec8ed617b8a0fc5e2b46968f","original_commit_id":"1b2b82e3dbeb1b53ec8ed617b8a0fc5e2b46968f","user":{"login":"slobodan-ilic","id":7716387,"avatar_url":"https://avatars1.githubusercontent.com/u/7716387?v=4","gravatar_id":"","url":"https://api.github.com/users/slobodan-ilic","html_url":"https://github.com/slobodan-ilic","followers_url":"https://api.github.com/users/slobodan-ilic/followers","following_url":"https://api.github.com/users/slobodan-ilic/following{/other_user}","gists_url":"https://api.github.com/users/slobodan-ilic/gists{/gist_id}","starred_url":"https://api.github.com/users/slobodan-ilic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/slobodan-ilic/subscriptions","organizations_url":"https://api.github.com/users/slobodan-ilic/orgs","repos_url":"https://api.github.com/users/slobodan-ilic/repos","events_url":"https://api.github.com/users/slobodan-ilic/events{/privacy}","received_events_url":"https://api.github.com/users/slobodan-ilic/received_events","type":"User","site_admin":false},"body":"@percious The joined strings are a conversion option that allowed me to complete the task.The size of BSON with integers just couldn't get reduced more than about 30M on my machine. I tried a number of different python types, including byte array and raw binary BSON, but they didn't help. I also tried using `None`s instead of `0`s, but that only helped insignificantly.\r\n\r\nThis is a kind of a problem for which I'd immediately ask a senior person (about the UInt in BSON, or the similar guideline). Especially so, since I didn't have prior experience with BSON or Mongo.\r\n\r\nAs for all possible categories encoding, it's done in [this](https://github.com/percious/crunch-fitness/pull/3/files#diff-0b0c428d576620398241ea0d50a1b6e7R135) part of the code. The `set` function does the reduction to unique values. Only the columns where `max_categories` number of categories is not exceeded are actually converted, so that we retain  numerical values (such as the salary).\r\n\r\nI'll do the changes to BSON, to use the UInt, and amend the commit. Thanks for the info :100: ","created_at":"2017-08-02T19:27:59Z","updated_at":"2017-08-02T19:27:59Z","html_url":"https://github.com/percious/crunch-fitness/pull/3#discussion_r130973164","pull_request_url":"https://api.github.com/repos/percious/crunch-fitness/pulls/3","_links":{"self":{"href":"https://api.github.com/repos/percious/crunch-fitness/pulls/comments/130973164"},"html":{"href":"https://github.com/percious/crunch-fitness/pull/3#discussion_r130973164"},"pull_request":{"href":"https://api.github.com/repos/percious/crunch-fitness/pulls/3"}}},"pull_request":{"url":"https://api.github.com/repos/percious/crunch-fitness/pulls/3","id":133278208,"html_url":"https://github.com/percious/crunch-fitness/pull/3","diff_url":"https://github.com/percious/crunch-fitness/pull/3.diff","patch_url":"https://github.com/percious/crunch-fitness/pull/3.patch","issue_url":"https://api.github.com/repos/percious/crunch-fitness/issues/3","number":3,"state":"open","locked":false,"title":"CR-TEST: Crunch DB Fitness (Slobodan Ilic)","user":{"login":"slobodan-ilic","id":7716387,"avatar_url":"https://avatars1.githubusercontent.com/u/7716387?v=4","gravatar_id":"","url":"https://api.github.com/users/slobodan-ilic","html_url":"https://github.com/slobodan-ilic","followers_url":"https://api.github.com/users/slobodan-ilic/followers","following_url":"https://api.github.com/users/slobodan-ilic/following{/other_user}","gists_url":"https://api.github.com/users/slobodan-ilic/gists{/gist_id}","starred_url":"https://api.github.com/users/slobodan-ilic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/slobodan-ilic/subscriptions","organizations_url":"https://api.github.com/users/slobodan-ilic/orgs","repos_url":"https://api.github.com/users/slobodan-ilic/repos","events_url":"https://api.github.com/users/slobodan-ilic/events{/privacy}","received_events_url":"https://api.github.com/users/slobodan-ilic/received_events","type":"User","site_admin":false},"body":"This PR represents the work that has been done on the assigned challenge (cr-db). The PR is opened as per request of @percious. All comments, suggestions and critiques are welcome.","created_at":"2017-07-31T15:49:29Z","updated_at":"2017-08-02T19:27:59Z","closed_at":null,"merged_at":null,"merge_commit_sha":"29e0268721bf5fe9342ae4d9378e4d94042a8881","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/percious/crunch-fitness/pulls/3/commits","review_comments_url":"https://api.github.com/repos/percious/crunch-fitness/pulls/3/comments","review_comment_url":"https://api.github.com/repos/percious/crunch-fitness/pulls/comments{/number}","comments_url":"https://api.github.com/repos/percious/crunch-fitness/issues/3/comments","statuses_url":"https://api.github.com/repos/percious/crunch-fitness/statuses/1b2b82e3dbeb1b53ec8ed617b8a0fc5e2b46968f","head":{"label":"slobodan-ilic:crunch-fitness-with-classes","ref":"crunch-fitness-with-classes","sha":"1b2b82e3dbeb1b53ec8ed617b8a0fc5e2b46968f","user":{"login":"slobodan-ilic","id":7716387,"avatar_url":"https://avatars1.githubusercontent.com/u/7716387?v=4","gravatar_id":"","url":"https://api.github.com/users/slobodan-ilic","html_url":"https://github.com/slobodan-ilic","followers_url":"https://api.github.com/users/slobodan-ilic/followers","following_url":"https://api.github.com/users/slobodan-ilic/following{/other_user}","gists_url":"https://api.github.com/users/slobodan-ilic/gists{/gist_id}","starred_url":"https://api.github.com/users/slobodan-ilic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/slobodan-ilic/subscriptions","organizations_url":"https://api.github.com/users/slobodan-ilic/orgs","repos_url":"https://api.github.com/users/slobodan-ilic/repos","events_url":"https://api.github.com/users/slobodan-ilic/events{/privacy}","received_events_url":"https://api.github.com/users/slobodan-ilic/received_events","type":"User","site_admin":false},"repo":{"id":98456258,"name":"crunch-fitness","full_name":"slobodan-ilic/crunch-fitness","owner":{"login":"slobodan-ilic","id":7716387,"avatar_url":"https://avatars1.githubusercontent.com/u/7716387?v=4","gravatar_id":"","url":"https://api.github.com/users/slobodan-ilic","html_url":"https://github.com/slobodan-ilic","followers_url":"https://api.github.com/users/slobodan-ilic/followers","following_url":"https://api.github.com/users/slobodan-ilic/following{/other_user}","gists_url":"https://api.github.com/users/slobodan-ilic/gists{/gist_id}","starred_url":"https://api.github.com/users/slobodan-ilic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/slobodan-ilic/subscriptions","organizations_url":"https://api.github.com/users/slobodan-ilic/orgs","repos_url":"https://api.github.com/users/slobodan-ilic/repos","events_url":"https://api.github.com/users/slobodan-ilic/events{/privacy}","received_events_url":"https://api.github.com/users/slobodan-ilic/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/slobodan-ilic/crunch-fitness","description":"Code test for burgeoning crunchers","fork":true,"url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness","forks_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/forks","keys_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/keys{/key_id}","collaborators_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/teams","hooks_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/hooks","issue_events_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/issues/events{/number}","events_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/events","assignees_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/assignees{/user}","branches_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/branches{/branch}","tags_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/tags","blobs_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/git/refs{/sha}","trees_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/git/trees{/sha}","statuses_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/statuses/{sha}","languages_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/languages","stargazers_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/stargazers","contributors_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/contributors","subscribers_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/subscribers","subscription_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/subscription","commits_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/commits{/sha}","git_commits_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/git/commits{/sha}","comments_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/comments{/number}","issue_comment_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/issues/comments{/number}","contents_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/contents/{+path}","compare_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/compare/{base}...{head}","merges_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/merges","archive_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/downloads","issues_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/issues{/number}","pulls_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/pulls{/number}","milestones_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/milestones{/number}","notifications_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/labels{/name}","releases_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/releases{/id}","deployments_url":"https://api.github.com/repos/slobodan-ilic/crunch-fitness/deployments","created_at":"2017-07-26T18:59:36Z","updated_at":"2017-07-26T18:59:38Z","pushed_at":"2017-07-31T12:13:59Z","git_url":"git://github.com/slobodan-ilic/crunch-fitness.git","ssh_url":"git@github.com:slobodan-ilic/crunch-fitness.git","clone_url":"https://github.com/slobodan-ilic/crunch-fitness.git","svn_url":"https://github.com/slobodan-ilic/crunch-fitness","homepage":null,"size":23413,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"percious:master","ref":"master","sha":"8131f65d20df23188ac554f37bc4b10f89881578","user":{"login":"percious","id":139328,"avatar_url":"https://avatars0.githubusercontent.com/u/139328?v=4","gravatar_id":"","url":"https://api.github.com/users/percious","html_url":"https://github.com/percious","followers_url":"https://api.github.com/users/percious/followers","following_url":"https://api.github.com/users/percious/following{/other_user}","gists_url":"https://api.github.com/users/percious/gists{/gist_id}","starred_url":"https://api.github.com/users/percious/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/percious/subscriptions","organizations_url":"https://api.github.com/users/percious/orgs","repos_url":"https://api.github.com/users/percious/repos","events_url":"https://api.github.com/users/percious/events{/privacy}","received_events_url":"https://api.github.com/users/percious/received_events","type":"User","site_admin":false},"repo":{"id":75997527,"name":"crunch-fitness","full_name":"percious/crunch-fitness","owner":{"login":"percious","id":139328,"avatar_url":"https://avatars0.githubusercontent.com/u/139328?v=4","gravatar_id":"","url":"https://api.github.com/users/percious","html_url":"https://github.com/percious","followers_url":"https://api.github.com/users/percious/followers","following_url":"https://api.github.com/users/percious/following{/other_user}","gists_url":"https://api.github.com/users/percious/gists{/gist_id}","starred_url":"https://api.github.com/users/percious/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/percious/subscriptions","organizations_url":"https://api.github.com/users/percious/orgs","repos_url":"https://api.github.com/users/percious/repos","events_url":"https://api.github.com/users/percious/events{/privacy}","received_events_url":"https://api.github.com/users/percious/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/percious/crunch-fitness","description":"Code test for burgeoning crunchers","fork":false,"url":"https://api.github.com/repos/percious/crunch-fitness","forks_url":"https://api.github.com/repos/percious/crunch-fitness/forks","keys_url":"https://api.github.com/repos/percious/crunch-fitness/keys{/key_id}","collaborators_url":"https://api.github.com/repos/percious/crunch-fitness/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/percious/crunch-fitness/teams","hooks_url":"https://api.github.com/repos/percious/crunch-fitness/hooks","issue_events_url":"https://api.github.com/repos/percious/crunch-fitness/issues/events{/number}","events_url":"https://api.github.com/repos/percious/crunch-fitness/events","assignees_url":"https://api.github.com/repos/percious/crunch-fitness/assignees{/user}","branches_url":"https://api.github.com/repos/percious/crunch-fitness/branches{/branch}","tags_url":"https://api.github.com/repos/percious/crunch-fitness/tags","blobs_url":"https://api.github.com/repos/percious/crunch-fitness/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/percious/crunch-fitness/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/percious/crunch-fitness/git/refs{/sha}","trees_url":"https://api.github.com/repos/percious/crunch-fitness/git/trees{/sha}","statuses_url":"https://api.github.com/repos/percious/crunch-fitness/statuses/{sha}","languages_url":"https://api.github.com/repos/percious/crunch-fitness/languages","stargazers_url":"https://api.github.com/repos/percious/crunch-fitness/stargazers","contributors_url":"https://api.github.com/repos/percious/crunch-fitness/contributors","subscribers_url":"https://api.github.com/repos/percious/crunch-fitness/subscribers","subscription_url":"https://api.github.com/repos/percious/crunch-fitness/subscription","commits_url":"https://api.github.com/repos/percious/crunch-fitness/commits{/sha}","git_commits_url":"https://api.github.com/repos/percious/crunch-fitness/git/commits{/sha}","comments_url":"https://api.github.com/repos/percious/crunch-fitness/comments{/number}","issue_comment_url":"https://api.github.com/repos/percious/crunch-fitness/issues/comments{/number}","contents_url":"https://api.github.com/repos/percious/crunch-fitness/contents/{+path}","compare_url":"https://api.github.com/repos/percious/crunch-fitness/compare/{base}...{head}","merges_url":"https://api.github.com/repos/percious/crunch-fitness/merges","archive_url":"https://api.github.com/repos/percious/crunch-fitness/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/percious/crunch-fitness/downloads","issues_url":"https://api.github.com/repos/percious/crunch-fitness/issues{/number}","pulls_url":"https://api.github.com/repos/percious/crunch-fitness/pulls{/number}","milestones_url":"https://api.github.com/repos/percious/crunch-fitness/milestones{/number}","notifications_url":"https://api.github.com/repos/percious/crunch-fitness/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/percious/crunch-fitness/labels{/name}","releases_url":"https://api.github.com/repos/percious/crunch-fitness/releases{/id}","deployments_url":"https://api.github.com/repos/percious/crunch-fitness/deployments","created_at":"2016-12-09T03:19:37Z","updated_at":"2016-12-13T23:56:02Z","pushed_at":"2017-07-31T15:49:30Z","git_url":"git://github.com/percious/crunch-fitness.git","ssh_url":"git@github.com:percious/crunch-fitness.git","clone_url":"https://github.com/percious/crunch-fitness.git","svn_url":"https://github.com/percious/crunch-fitness","homepage":null,"size":22091,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":5,"mirror_url":null,"open_issues_count":1,"forks":5,"open_issues":1,"watchers":0,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/percious/crunch-fitness/pulls/3"},"html":{"href":"https://github.com/percious/crunch-fitness/pull/3"},"issue":{"href":"https://api.github.com/repos/percious/crunch-fitness/issues/3"},"comments":{"href":"https://api.github.com/repos/percious/crunch-fitness/issues/3/comments"},"review_comments":{"href":"https://api.github.com/repos/percious/crunch-fitness/pulls/3/comments"},"review_comment":{"href":"https://api.github.com/repos/percious/crunch-fitness/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/percious/crunch-fitness/pulls/3/commits"},"statuses":{"href":"https://api.github.com/repos/percious/crunch-fitness/statuses/1b2b82e3dbeb1b53ec8ed617b8a0fc5e2b46968f"}}}}
{"actor":{"display_login":"slobodan-ilic"}}