Skip to content

Conversation

@ferenc-rad
Copy link
Collaborator

@ferenc-rad ferenc-rad commented Oct 31, 2025

Add a new commad line option that enables turning off click's standalone_mode, thus allowing collecting memory traces with viztracer:
https://click.palletsprojects.com/en/stable/api/#click.Command.main
https://viztracer.readthedocs.io/en/latest/

Checklist

  • I have added tests that prove my fix is effective or that my feature works (discussed: no need for tests for this developer feature)
  • New and existing unit tests pass locally with my changes
  • I have made corresponding changes to the documentation

@ferenc-rad ferenc-rad requested a review from dkazanc October 31, 2025 14:18
@dkazanc dkazanc requested a review from yousefmoazzam October 31, 2025 14:46
@yousefmoazzam yousefmoazzam marked this pull request as ready for review November 3, 2025 12:32
Copy link
Collaborator

@yousefmoazzam yousefmoazzam left a comment

Choose a reason for hiding this comment

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

I had a go at seeing if this could be done through click; alas, I think you're right that this has to be done outside of click.

I tried to sneakily give the main() function a bool parameter called standalone_mode and then define a@click.option on it. The flag works and is available on python -m httomo rather than on python -m httomo run, but it doesn't change the behaviour - I think this was simply defining a flag that happened to have the same name, but didn't actually hook into click's standalone_mode parameter.

On the note about tests, given that this is essentially a secret flag for developers only, and that adding a test would probably require some fiddling with __main__.py, I think I'm content to say that as long as we know that all other CLI tests pass, then that's good enough.

The IRIS CI is in a bit of a weird state due to scheduled maintenance occurring over the weekend, but I can confirm that the tests pass locally, and running pipelines without this flag seems unaffected.

Please feel free to update the branch as needed, and then I think this is fine to merge, thanks! 🙂

@yousefmoazzam yousefmoazzam merged commit 05bca51 into main Nov 3, 2025
5 of 6 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.

3 participants