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.

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

[MRG] Add more information about pydicom purpose #1709

Merged
merged 2 commits into from Oct 8, 2022

Conversation

mrbean-bremen
Copy link
Member

  • slightly adapt the issue templates

I added a couple of sentences to the README as a starting point for the discussed changes.
We probably also need some additions in the documentation itself...

- slightly adapt the issue templates
@codecov
Copy link

codecov bot commented Oct 6, 2022

Codecov Report

Merging #1709 (11dd56c) into master (ef62ff7) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1709      +/-   ##
==========================================
+ Coverage   97.58%   97.60%   +0.01%     
==========================================
  Files          66       66              
  Lines       10744    10744              
==========================================
+ Hits        10485    10487       +2     
+ Misses        259      257       -2     
Impacted Files Coverage Δ
pydicom/filebase.py 99.19% <0.00%> (+1.61%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

README.md Outdated
Comment on lines 134 to 137
If you have a software package based on *pydicom* that you want to be more
tightly integrated into the *pydicom* ecosystem, you may consider to apply
for transferring it into the *pydicom* organization. You can do so by creating
a general discussion item with a short description of the module.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this - I also wanted to document the basic reqiurements for such a transfer somewhere else, but first wanted to know if we want to add this at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also not sure about this - I think by adding projects to the pydicom organization, we are essentially endorsing certain libraries over others. The only benefit is to users, in helping them find libraries they might not otherwise have known about.

I'd suggest we leave this out for now, and leave it to the pydicom owners to offer a move to the organization, in cases where a clear 'winner' exists that people should know about, that doesn't already have a strong management team under its own organization / individual account.


Examples are [pynetdicom](https://github.com/pydicom/pynetdicom), which
is a Python library for DICOM networking, and [deid](https://github.com/pydicom/deid),
which supports the anonymization of DICOM files.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added linebreaks to avoid too much horizontal scrolling, these are ignored.

Copy link
Member

@darcymason darcymason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these changes, these were due for some updating. A couple of comments...

README.md Outdated
Note that *pydicom* is a general-purpose DICOM framework concerned with
reading and writing DICOM datasets, and does not handle the specifics
of individual SOP classes or other aspects of DICOM on purpose. Other
libraries both inside and outside the [pydicom organization](https://github.com/pydicom)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "on purpose" here sounds a bit odd to me. Perhaps needs more explanation, like '...or other aspects of DICOM, in order to keep the project manageable'.

README.md Outdated
Comment on lines 134 to 137
If you have a software package based on *pydicom* that you want to be more
tightly integrated into the *pydicom* ecosystem, you may consider to apply
for transferring it into the *pydicom* organization. You can do so by creating
a general discussion item with a short description of the module.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also not sure about this - I think by adding projects to the pydicom organization, we are essentially endorsing certain libraries over others. The only benefit is to users, in helping them find libraries they might not otherwise have known about.

I'd suggest we leave this out for now, and leave it to the pydicom owners to offer a move to the organization, in cases where a clear 'winner' exists that people should know about, that doesn't already have a strong management team under its own organization / individual account.

@mrbean-bremen
Copy link
Member Author

I'd suggest we leave this out for now, and leave it to the pydicom owners to offer a move to the organization

Agreed. As I wrote, I wasn't sure about this, and this came mostly from the transfer of pyfakefs to pytest-dev I was involved with (now finished). They have some rules and guidelines for this, but than they have about 60 repositories in the organization, and there are over a 1000 pytest plugins that are potential candidates, so I guess they need this more than we do.

@mrbean-bremen mrbean-bremen changed the title [WIP] Add more information about pydicom purpose [MRG] Add more information about pydicom purpose Oct 8, 2022
@mrbean-bremen
Copy link
Member Author

Maybe we will take this as a start and add more later if we thiunk of something (or so I understood your approval 😄 ).

@darcymason
Copy link
Member

Maybe we will take this as a start and add more later if we thiunk of something (or so I understood your approval 😄 ).

Yes, I think it is a good start. It certainly helps clear up one kind of common request for pydicom features.

@darcymason darcymason merged commit 16fd77d into pydicom:master Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants