Skip to content

Conversation

@darshad-github
Copy link
Contributor

@darshad-github darshad-github commented Mar 30, 2023

Proposed Changes

This pull request fixes an issue spotted when running a test extraction. The problem occurs when UIDs present in the PACS data that are used to generate the extraction output paths <StudyInstanceUID>/<SeriesInstanceUID>/... start with a dot. This indicates a hidden directory in Linux causing some directories to appear "missing" from the extraction output unless hidden files are shown.

To fix this issue, the leading periods are now removed when generating the output path in the DefaultProjectPathResolver class. Additionally, a new test case has been added to the DefaultProjectPathResolverTest class to ensure that hidden directories are handled correctly.

  • Branch: fix/cohort-extractor-output-paths
    • Modified DefaultProjectPathResolver.cs to remove leading periods when generating the output path
    • Modified DefaultProjectPathResolverTest.cs to include a new test case for hidden directories

Types of changes

What types of changes does your code introduce? Tick all that apply.

  • Bugfix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation-Only Update (if none of the other choices apply)
    • In this case, ensure that the message of the head commit from the source branch is prefixed with [skip ci]

Checklist

By opening this PR, I confirm that I have:

  • Reviewed the contributing guidelines for this repository
  • Ensured that the PR branch is in sync with the target branch (i.e. it is automatically merge-able)
  • Updated any relevant API documentation
  • Created or updated any tests if relevant
  • Created a news file
    • NOTE: This must include any changes to any of the following files: default.yaml, any of the RabbitMQ server configurations, GlobalOptions.cs
  • Listed myself in the CONTRIBUTORS file 🚀
  • Requested a review by one of the repository maintainers

Issues

If relevant, tag any issues that are expected to be resolved with this PR. E.g.:

@darshad-github darshad-github self-assigned this Mar 30, 2023
@darshad-github darshad-github requested a review from rkm March 30, 2023 23:27
@codecov
Copy link

codecov bot commented Mar 30, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (811be8a) 67.87% compared to head (4e68e8f) 67.88%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1506   +/-   ##
=======================================
  Coverage   67.87%   67.88%           
=======================================
  Files         185      185           
  Lines        6544     6546    +2     
=======================================
+ Hits         4442     4444    +2     
  Misses       2102     2102           
Impacted Files Coverage Δ
...ProjectPathResolvers/DefaultProjectPathResolver.cs 95.23% <100.00%> (+0.50%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@rkm rkm left a comment

Choose a reason for hiding this comment

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

Looks great - just need to change the name of the news file to match the PR (not the issue number)

@darshad-github darshad-github requested a review from rkm March 31, 2023 15:05
@rkm
Copy link
Member

rkm commented Apr 3, 2023

@darshad-github feel free to hit the big green "merge" button! 😄

@darshad-github darshad-github merged commit 15529cb into master Apr 3, 2023
@rkm rkm deleted the fix/cohort-extractor-output-paths branch June 28, 2023 07:59
rkm added a commit that referenced this pull request Oct 4, 2024
rkm added a commit that referenced this pull request Oct 10, 2024
rkm added a commit that referenced this pull request Oct 10, 2024
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.

"Hidden" files in extraction output

3 participants