{"action":"created","issue":{"url":"https://api.github.com/repos/pydicom/pydicom/issues/660","repository_url":"https://api.github.com/repos/pydicom/pydicom","labels_url":"https://api.github.com/repos/pydicom/pydicom/issues/660/labels{/name}","comments_url":"https://api.github.com/repos/pydicom/pydicom/issues/660/comments","events_url":"https://api.github.com/repos/pydicom/pydicom/issues/660/events","html_url":"https://github.com/pydicom/pydicom/pull/660","id":333238460,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk1NTAxMDgy","number":660,"title":"[WIP] DA, TM, DT repval fix","user":{"login":"volrath","id":39625,"node_id":"MDQ6VXNlcjM5NjI1","avatar_url":"https://avatars1.githubusercontent.com/u/39625?v=4","gravatar_id":"","url":"https://api.github.com/users/volrath","html_url":"https://github.com/volrath","followers_url":"https://api.github.com/users/volrath/followers","following_url":"https://api.github.com/users/volrath/following{/other_user}","gists_url":"https://api.github.com/users/volrath/gists{/gist_id}","starred_url":"https://api.github.com/users/volrath/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/volrath/subscriptions","organizations_url":"https://api.github.com/users/volrath/orgs","repos_url":"https://api.github.com/users/volrath/repos","events_url":"https://api.github.com/users/volrath/events{/privacy}","received_events_url":"https://api.github.com/users/volrath/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-06-18T11:54:45Z","updated_at":"2018-07-01T15:37:20Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/pydicom/pydicom/pulls/660","html_url":"https://github.com/pydicom/pydicom/pull/660","diff_url":"https://github.com/pydicom/pydicom/pull/660.diff","patch_url":"https://github.com/pydicom/pydicom/pull/660.patch"},"body":"<!--\r\nThanks for contributing a pull request! Please ensure you have taken a look at\r\nthe contribution guidelines: https://github.com/pydicom/pydicom/blob/master/CONTRIBUTING.md#contributing-pull-requests\r\n-->\r\n#### Reference Issue\r\n<!-- Example: Fixes #1234 -->\r\n\r\nNone, afaict.\r\n\r\n\r\n#### What does this implement/fix? Explain your changes.\r\n<!--\r\nPlease summarize the key points of the reference issue, problem or contribution\r\nto facilitate reviewing task. Of course reviewers can always refer to the\r\noriginal issue but facilitating the reviewing process is much appreciated.\r\n-->\r\n\r\nAdds `__repr__` method to `DA`, `TM`, and `DT` values, to make them symmetrical to `IS` and `DS`.\r\n\r\nThis allows consumers to be able to obtain the original string as the representation of the `DA`/`TM`/`DT` object, instead of something like: `DA(2018, 6, 18)`\r\n\r\n#### Any other comments?\r\n<!--\r\n-->\r\n\r\nWhile going through this issue, I also notice that the `DataElement.repval` property tries to use the `original_string` attribute in order to print out the representation string, before falling back to `repr(self.value)`\r\n\r\nThis, on its own, is fine. However, as far as I could tell, `original_value` is not used/set in `DataElement` instances, but in value instances (`IS`, `DSfloat`, ...) so the `elif` clause in line `pydicom/dataelem.py:307` is never true.\r\n\r\nSince I'm not sure I'm missing something here, I just added an extra if clause to the `repval` property, that tries to use the data element value's original string.\r\n\r\nIf this is indeed a bug, I'm happing to fix it. Also, if checking `DataElement.original_string` is intentional, I can remove the extra if clause that I added and just rely on the value's `__repr__` method.\r\n\r\n<!--\r\nPlease be aware that we are a loose team of volunteers so patience is\r\nnecessary; assistance handling other issues is very welcome. We value\r\nall user contributions, no matter how minor they are. If we are slow to\r\nreview, either the pull request needs some benchmarking, tinkering,\r\nconvincing, etc. or more likely the reviewers are simply busy. In either\r\ncase, we ask for your understanding during the review process.\r\n\r\nThanks for contributing!\r\n-->\r\n"},"comment":{"url":"https://api.github.com/repos/pydicom/pydicom/issues/comments/401614364","html_url":"https://github.com/pydicom/pydicom/pull/660#issuecomment-401614364","issue_url":"https://api.github.com/repos/pydicom/pydicom/issues/660","id":401614364,"node_id":"MDEyOklzc3VlQ29tbWVudDQwMTYxNDM2NA==","user":{"login":"darcymason","id":61110,"node_id":"MDQ6VXNlcjYxMTEw","avatar_url":"https://avatars3.githubusercontent.com/u/61110?v=4","gravatar_id":"","url":"https://api.github.com/users/darcymason","html_url":"https://github.com/darcymason","followers_url":"https://api.github.com/users/darcymason/followers","following_url":"https://api.github.com/users/darcymason/following{/other_user}","gists_url":"https://api.github.com/users/darcymason/gists{/gist_id}","starred_url":"https://api.github.com/users/darcymason/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/darcymason/subscriptions","organizations_url":"https://api.github.com/users/darcymason/orgs","repos_url":"https://api.github.com/users/darcymason/repos","events_url":"https://api.github.com/users/darcymason/events{/privacy}","received_events_url":"https://api.github.com/users/darcymason/received_events","type":"User","site_admin":false},"created_at":"2018-07-01T15:37:20Z","updated_at":"2018-07-01T15:37:20Z","author_association":"MEMBER","body":"Hmm, I think the test should never have happened ... I did a search through the whole code base and I don't think that `<dataelem>.original_string` is used anywhere else (other than the test) in pydicom.  And as I mentioned earlier, it is in danger of getting out of sync with the `<dataelem>.value.original_string`.   I doubt that it has been used in user code, so I think we could just remove it (as the PR already does) and note that in the next release notes.  However, if someone speaks up to the need, we could add a property to redirect it to `<dataelem>.value.original_string` with a deprecation warning."}}
{"actor":{"display_login":"darcymason"}}