{"action":"created","issue":{"url":"https://api.github.com/repos/dimagi/jsonobject/issues/131","repository_url":"https://api.github.com/repos/dimagi/jsonobject","labels_url":"https://api.github.com/repos/dimagi/jsonobject/issues/131/labels{/name}","comments_url":"https://api.github.com/repos/dimagi/jsonobject/issues/131/comments","events_url":"https://api.github.com/repos/dimagi/jsonobject/issues/131/events","html_url":"https://github.com/dimagi/jsonobject/pull/131","id":141729288,"number":131,"title":"fix exclude_if_none to do what it should","user":{"login":"KeithTsui","id":633965,"avatar_url":"https://avatars.githubusercontent.com/u/633965?v=3","gravatar_id":"","url":"https://api.github.com/users/KeithTsui","html_url":"https://github.com/KeithTsui","followers_url":"https://api.github.com/users/KeithTsui/followers","following_url":"https://api.github.com/users/KeithTsui/following{/other_user}","gists_url":"https://api.github.com/users/KeithTsui/gists{/gist_id}","starred_url":"https://api.github.com/users/KeithTsui/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/KeithTsui/subscriptions","organizations_url":"https://api.github.com/users/KeithTsui/orgs","repos_url":"https://api.github.com/users/KeithTsui/repos","events_url":"https://api.github.com/users/KeithTsui/events{/privacy}","received_events_url":"https://api.github.com/users/KeithTsui/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"dannyroberts","id":137212,"avatar_url":"https://avatars.githubusercontent.com/u/137212?v=3","gravatar_id":"","url":"https://api.github.com/users/dannyroberts","html_url":"https://github.com/dannyroberts","followers_url":"https://api.github.com/users/dannyroberts/followers","following_url":"https://api.github.com/users/dannyroberts/following{/other_user}","gists_url":"https://api.github.com/users/dannyroberts/gists{/gist_id}","starred_url":"https://api.github.com/users/dannyroberts/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dannyroberts/subscriptions","organizations_url":"https://api.github.com/users/dannyroberts/orgs","repos_url":"https://api.github.com/users/dannyroberts/repos","events_url":"https://api.github.com/users/dannyroberts/events{/privacy}","received_events_url":"https://api.github.com/users/dannyroberts/received_events","type":"User","site_admin":false},"milestone":null,"comments":2,"created_at":"2016-03-17T23:05:18Z","updated_at":"2016-03-18T13:43:40Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/dimagi/jsonobject/pulls/131","html_url":"https://github.com/dimagi/jsonobject/pull/131","diff_url":"https://github.com/dimagi/jsonobject/pull/131.diff","patch_url":"https://github.com/dimagi/jsonobject/pull/131.patch"},"body":"Great library, but two things I wanted to address in the pull request:\r\n1) Normally wouldn't want null values to be all over the json and would prefer the field to just be absent, so changing the default for exclude_if_none=True\r\n2) fix the exclude function to actually do a None check because I would want a boolean property to print out false.\r\n\r\nThanks!\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/dimagi/jsonobject/issues/comments/198366405","html_url":"https://github.com/dimagi/jsonobject/pull/131#issuecomment-198366405","issue_url":"https://api.github.com/repos/dimagi/jsonobject/issues/131","id":198366405,"user":{"login":"dannyroberts","id":137212,"avatar_url":"https://avatars.githubusercontent.com/u/137212?v=3","gravatar_id":"","url":"https://api.github.com/users/dannyroberts","html_url":"https://github.com/dannyroberts","followers_url":"https://api.github.com/users/dannyroberts/followers","following_url":"https://api.github.com/users/dannyroberts/following{/other_user}","gists_url":"https://api.github.com/users/dannyroberts/gists{/gist_id}","starred_url":"https://api.github.com/users/dannyroberts/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dannyroberts/subscriptions","organizations_url":"https://api.github.com/users/dannyroberts/orgs","repos_url":"https://api.github.com/users/dannyroberts/repos","events_url":"https://api.github.com/users/dannyroberts/events{/privacy}","received_events_url":"https://api.github.com/users/dannyroberts/received_events","type":"User","site_admin":false},"created_at":"2016-03-18T13:43:40Z","updated_at":"2016-03-18T13:43:40Z","body":"@KeithTsui thanks for the contribution. I think both of these changes have real merit, and I want to help get them in, but it's going to require a bit more than the basic implementation you've done here. As @czue noted, this library is heavily tested on exact inputs and outputs, and the changes you're suggesting are pretty far reaching. We aim heavily to be backwards compatible `couchdbkit` as well as previous versions of this library (`jsonobject`).\r\n\r\nFor both of your changes, I think there are good ways to make them backwards compatible.\r\n\r\nFor (2), I would add an additional argument called `exclude_if_is_none` that has the behavior you are looking for. Now, that's pretty confusing and you're right that `exclude_if_none` was a poor choice of words. So I'd add a deprecation warning if that param is set to `True`, letting the person writing the client code to use `exclude_if_falsey` instead. Both `exclude_if_none` and `exclude_if_falsey` will live side-by-side and do the same thing, but `exclude_if_none` will have a deprecation warning and `exclude_if_falsey` will be preferred. Then, if you want your behavior you can write:\r\n\r\n```python\r\nBooleanProperty(exclude_if_is_none=True)\r\n```\r\n\r\ncurrent code using `exclude_if_none` will continue to work (with a deprecation warning), and new code that wants that behavior can use\r\n\r\n```python\r\nBooleanProperty(exclude_if_falsey=True)\r\n```\r\n\r\nThat's for (2). For (1) I suggest having someone who wants to override the default explicitly declare that at the beginning of their code. So that would look like:\r\n\r\n```python\r\nimport jsonobject\r\njsonobject.configure_defaults(exclude_if_is_none=True)\r\n```\r\n\r\nThis would have to be called before any `jsonobject.JsonObject` classes were declared.\r\n\r\nThen inside `JsonProperty.__init__`, you could have `exclude_if_is_none=None` in the params and then have something like\r\n\r\n```python\r\nif exclude_if_is_none is None:\r\n    exclude_if_is_none = get_default_configuration().exclude_if_is_none\r\n```\r\n\r\nCurrently, neither `get_default_configuration` nor `configure_defaults` exist, so that would have to be part of the change. Since there are plenty of default behaviors that one might want to configure, having that infrastructure in this library will make it accessible to a wider audience and I think would overall be a very healthy thing.\r\n\r\nPlease, feel free to reach out for more information. If you're interested in taking on this larger task, I'd be happy to spend the time to make sure you're able to do it well and answer questions you might have along the way."}}