Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Alternative construction of objects from existing data sets #93

Merged
merged 208 commits into from Aug 31, 2021
Merged

Alternative construction of objects from existing data sets #93

merged 208 commits into from Aug 31, 2021

Conversation

CPBridge
Copy link
Collaborator

Development branch combining work from #69 and #74 (SR and SEG decoding) into a single PR

CPBridge and others added 30 commits April 8, 2021 12:59
@CPBridge
Integrate new pydicom DS formatting
b597135
@hackermd
Fix recording of evidence in structured reports
d9784a7
@hackermd
Increase package version
f2e8ddf
@hackermd
Assert that evidence is provided for references
c91d00b
@hackermd
Use sets for comparison and avoid duplicates
8e6ad62
@hackermd
Fix mypy errors
58351d6
@hackermd
Add test for report referencing multiple studies
dbe4c77
@hackermd
Remove import statement for unused variable
7284f0c
@hackermd
Add properties to content items
c24d0e0
@hackermd
Fix recording of evidence in structured reports
466819c
@hackermd
Use sets for comparison and avoid duplicates
68af856
@hackermd
Fix mypy errors
0665672
@hackermd
Add test for report referencing multiple studies
1e0bb4f
@hackermd
Add properties to content items
9205175
@hackermd
Merge branch 'feat/sr-content-decoding' of github:mghcomputationalpat…
da95df2 
…hology/highdicom into feat/sr-content-decoding
@hackermd
Use datetime workaround for python 3.6 support
97622c7
@hackermd
Incomplete implementation of image library
0bb3660
@hackermd
Add proporties and methods on SR templates
0be93c3
@hackermd
Update type of content item property return value
d3b1fde
@hackermd
Fix alternative construction of sr documents
ce92631
Typo in exception message
7ac37ab
@hackermd
Fix recording of evidence in structured reports
82f8520
@hackermd
Use sets for comparison and avoid duplicates
7340f25
@hackermd
Fix mypy errors
e67a7a0
@hackermd
Add test for report referencing multiple studies
503118c
@hackermd
Add properties to content items
76eb2e8
@hackermd
Fix recording of evidence in structured reports
cda1d75
@hackermd
Use sets for comparison and avoid duplicates
0344ea2
@hackermd
Fix mypy errors
56b674e
@hackermd
Add test for report referencing multiple studies
9986b3e
@cgorman
Copy link
Member

cgorman commented Aug 10, 2021

map is a Python built-in. Therefore having a sub-package called map could be a really bad idea.
Suggest we change to pmap before merging to master. Thoughts?

Yes, if imported that way, it can cause trouble. We chose map to allow for implementation of other "maps" under that namespace. However, it's unclear whether we will ever implement them and risking overriding a built-in symbol is not nice. Linters will also highlight it, which may be confusing. I would be fine with renaming the package to pmaps or pm.

Ah yeah, I feel like you told me about this one time and I forgot... I vote for pm for consistency

@hackermd
Copy link
Collaborator

@CPBridge renamed package to pm with ac711e3

hackermd and others added 4 commits August 10, 2021 15:12
@hackermd @CPBridge
Update src/highdicom/sr/templates.py
04acd10 
Co-authored-by: Chris Bridge <chrisbridge44@googlemail.com>
@hackermd @CPBridge
Apply suggestions from code review
4ac4de8 
Co-authored-by: Chris Bridge <chrisbridge44@googlemail.com>
@hackermd
Don't add empty content sequence
79aa9fa
@hackermd
Merge branch 'dev' of github:mghcomputationalpathology/highdicom into…
79dd362 
… dev
@hackermd hackermd mentioned this pull request Aug 11, 2021
Open
@hackermd hackermd mentioned this pull request Aug 12, 2021
Closed
Christopher Bridge and others added 13 commits August 12, 2021 09:04
@hackermd
Copy link
Collaborator

@CPBridge unit tests are now passing with the latest pydicom 2.2.1 version. Thanks to @darcymason for creating the patch release!

There are a few remaining mypy issues (in particular pydicom/pydicom#1454), but sorting those out may require a bit more work on pydicom and I'd suggest ignoring them for the time being.

@hackermd hackermd merged commit bdd2a9d into master Aug 31, 2021
@hackermd hackermd deleted the dev branch August 31, 2021 21:40
@hackermd hackermd changed the title Highdicom 0.9.0 Alternative construction of objects from existing data sets Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hackermd hackermd hackermd approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@CPBridge @hackermd @cgorman @seandoyle