Skip to content

support adcmod sibling directories#2

Open
joefutrelle wants to merge 4 commits into
mainfrom
adcmod
Open

support adcmod sibling directories#2
joefutrelle wants to merge 4 commits into
mainfrom
adcmod

Conversation

@joefutrelle

Copy link
Copy Markdown
Collaborator

This pull request adds support for using corrected ADC files (with the .adc.mod extension) from a sibling adcmod directory, preferring them over raw .adc files when available. It updates both synchronous and asynchronous code paths to resolve the appropriate ADC file and includes comprehensive tests for this new behavior.

ADC file resolution enhancements:

  • Added logic to search for corrected ADC files (.adc.mod) in an adcmod sibling directory, falling back to the raw .adc if none is found. This is implemented in the new _sync_resolve_adc_path and _async_resolve_adc_path functions, which are used throughout the codebase for both synchronous and asynchronous file resolution.
  • Updated the paths and list methods in both SyncIfcbDataDirectory and AsyncIfcbDataDirectory to use the new resolution logic, ensuring that the returned ADC path points to the corrected file if it exists. [1] [2] [3] [4]

Testing improvements:

  • Added a new test suite (tests/test_fileset.py) that covers various scenarios for ADC file resolution, including presence/absence of adcmod files, directory structures, and end-to-end validation that corrected ADC files are actually used in downstream processing.

@joefutrelle
joefutrelle requested a review from Copilot July 15, 2026 17:33
@joefutrelle joefutrelle self-assigned this Jul 15, 2026
@joefutrelle
joefutrelle requested a review from johnwaalsh July 15, 2026 17:34
@joefutrelle joefutrelle changed the title support admod sibling directories support adcmod sibling directories Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds support for preferring corrected ADC files (.adc.mod) located in an adcmod directory when resolving fileset paths, across both synchronous and asynchronous directory APIs.

Changes:

  • Introduces shared sync/async ADC path resolution helpers that search for .adc.mod candidates and fall back to the raw .adc.
  • Updates SyncIfcbDataDirectory and AsyncIfcbDataDirectory paths() and list() to use the new resolution logic.
  • Adds a dedicated test suite validating .adc.mod preference behavior, including an end-to-end I-style bin read-path check.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/ifcbkit/fileset.py Adds adcmod-aware ADC resolution helpers and wires them into sync/async directory paths()/list() outputs.
tests/test_fileset.py Adds coverage for adcmod resolution scenarios and end-to-end validation that corrected ADC content is used.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_fileset.py
Comment thread tests/test_fileset.py
Comment thread tests/test_fileset.py
Comment thread tests/test_fileset.py
Comment thread src/ifcbkit/fileset.py

@johnwaalsh johnwaalsh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me!

@joefutrelle

Copy link
Copy Markdown
Collaborator Author

@johnwaalsh I found a problem with behavior and fixed it so requesting another look.

@joefutrelle
joefutrelle requested a review from johnwaalsh July 16, 2026 16:06

@johnwaalsh johnwaalsh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That behavior change makes sense to me. Good to avoid checking paths that we don't need to.

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.

3 participants