Skip to content

Custom include statements#236

Merged
ryanhill1 merged 12 commits intomainfrom
luke_include
Aug 1, 2025
Merged

Custom include statements#236
ryanhill1 merged 12 commits intomainfrom
luke_include

Conversation

@LukeAndreesen
Copy link
Copy Markdown
Collaborator

Custom include file support (issue)

A basic implementation of processing custom .inc and .qasm "include" files in QASM3 programs. This implementation simply inserts the contents of included files directly into the file where they are referenced prior to parsing the program. This is a preliminary, naive approach - any feedback would be appreciated.

Summary of Changes

  • Created _process_include_statements in entrypoint.py, which simply iterates through included files, searches for the included file in user's directory, then replaces the include statement with the content of the file.
    • We take this approach rather than attempting to switch parser context because included files are not necessarily valid QASM programs that can be parsed
  • Created additional test circuits for custom included file workflow, and corresponding tests in test_entrypoint.py
  • Currently, recursive inclusion processing is not implemented, ie. if an included file includes another custom file, that is not currently processed - will implement after preliminary feedback on approach.

@github-actions
Copy link
Copy Markdown
Contributor

👋 Hey there! It looks like the changelog might need an update.

Please take a moment to edit the CHANGELOG.md with:

  • A brief, one-to-two sentence summary of your changes.
  • A link back to this PR for reference.
  • (Optional) A small working example if you've added new features.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 25, 2025

Codecov Report

❌ Patch coverage is 94.18605% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pyqasm/preprocess.py 95.06% 4 Missing ⚠️
src/pyqasm/entrypoint.py 80.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@LukeAndreesen LukeAndreesen marked this pull request as draft July 25, 2025 17:52
@LukeAndreesen LukeAndreesen marked this pull request as ready for review July 25, 2025 19:08
Comment thread src/pyqasm/preprocess.py Outdated
Comment thread src/pyqasm/preprocess.py Outdated
Comment thread src/pyqasm/preprocess.py Outdated
Comment thread src/pyqasm/preprocess.py Outdated
Comment thread tests/qasm3/test_entrypoint.py Outdated
Comment thread src/pyqasm/preprocess.py Outdated
Comment thread src/pyqasm/preprocess.py Outdated
Copy link
Copy Markdown
Member

@TheGupta2012 TheGupta2012 left a comment

Choose a reason for hiding this comment

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

Hi @LukeAndreesen , thanks for the changes! It looks good overall, some comments about tests and minor refactoring.

Also, if you wanna give it a try, the recursive approach for multiple includes can be implemented using a stack as well.

@LukeAndreesen
Copy link
Copy Markdown
Collaborator Author

Hey @TheGupta2012 , thanks for the feedback! I'll work on getting those changes made and implement the recursive approach!

…cases, moved preprocessing testing to new file
@LukeAndreesen
Copy link
Copy Markdown
Collaborator Author

LukeAndreesen commented Jul 29, 2025

Hey @TheGupta2012 , I just pushed over a fresh version of the pre-processing routine, which now uses a stack-based approach to recursively handle nested inclusions. I ended up needing to change around my last commit quite a bit to work out some of the details of this nested include handling, so apologies for a lot of fresh code.

I tried to get things to a pretty clean and comprehensible place, but the code definitely still feels a bit confusing and perhaps not as efficient as it could be, though it does work. As such, any feedback on the code, or suggestions on a better approach to take, would be greatly appreciated!

Also, please let me know if you have any suggestions on my current testing format - I know I created quite a few .qasm and .inc files for testing, but am happy to clean this up / reformat tests if needed.

Thanks so much,
Luke

Comment thread src/pyqasm/preprocess.py Outdated
Comment thread src/pyqasm/preprocess.py Outdated
Comment thread src/pyqasm/preprocess.py Outdated
Comment thread src/pyqasm/preprocess.py Outdated
Comment thread src/pyqasm/preprocess.py
Comment thread tests/qasm3/resources/qasm/custom_include/include_sandwiched_ref.qasm Outdated
Comment thread tests/qasm3/test_preprocess.py Outdated
Copy link
Copy Markdown
Member

@TheGupta2012 TheGupta2012 left a comment

Choose a reason for hiding this comment

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

Nice work @LukeAndreesen ! I like the IncludeContext and stack based approach. Gave a few comments but looks pretty close to merging

@LukeAndreesen
Copy link
Copy Markdown
Collaborator Author

Hey @TheGupta2012 , I've made some changes to ensure the insertions occur at the line of the include statement in the original script - this ended up being easier by using a recursive call rather than stack approach to avoid reading files multiple times; hopefully the logic here should be a bit clearer to follow.

Comment thread tests/qasm3/test_preprocess.py
Copy link
Copy Markdown
Member

@TheGupta2012 TheGupta2012 left a comment

Choose a reason for hiding this comment

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

Great work @LukeAndreesen ! The recursive approach sure looks a lot cleaner. Once you add the qasm2 tests, I think the code should be good to merge.

Copy link
Copy Markdown
Member

@TheGupta2012 TheGupta2012 left a comment

Choose a reason for hiding this comment

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

Lgtm

@LukeAndreesen LukeAndreesen requested a review from ryanhill1 August 1, 2025 14:21
@ryanhill1 ryanhill1 changed the title Luke include Custom include statements Aug 1, 2025
@ryanhill1 ryanhill1 merged commit b1e9d18 into main Aug 1, 2025
21 checks passed
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.

4 participants