-
Notifications
You must be signed in to change notification settings - Fork 399
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
Add -c
option: program passed as string
#61
Conversation
Hi, some tests failed because I changed output to the console. didn't expect it to be tested :)
I can fix the tests, but see if in general this looks fine to you |
Hi @tal66 and thanks for your contribution! Two things: can you add a test for this in the test suite. Please, check the format of other CLI-related tests. Also, per our contribution licensing documentation, would you mind adding a That should be as easy as doing: $ git commit --amend -C HEAD -s
$ git push -f from a checkout of this branch. But for now, don't worry about it (even less if you used the GitHub UI :) ) |
I'd prefer a different approach here. Instead of writing the command to a file, I'd prefer to add another case to memray/src/memray/commands/run.py Lines 52 to 55 in 73e563f
That's where we currently handle either running a file or running a module. I'd add another case for running a command, and do: if args.cmd:
exec(args.cmd, {"__name__": "__main__"})
elif args.run_as_module:
runpy.run_module(args.script, run_name="__main__", alter_sys=True)
else:
runpy.run_path(args.script, run_name="__main__") |
Hi, can you approve running the workflows on the last push? (used --force) |
Done! |
Use as follows: `memray run -c '<write python commands here, or paste py file contents. then close the quote>'` Signed-off-by: tal66 <[email protected]>
I just changed the signoff now, that was the only problem in the checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking so long to review this. This looks very good to me, though I have a few thoughts about it.
I'm happy to make these changes myself on top of what has already been committed here, but I'm curious to hear other people's thoughts on them.
Co-authored-by: Matt Wozniski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me - thank you for the contribution, @tal66!
Hi, this closes #44.
can now run:
memray run -c 'print("my python commands")'
-> a py file will be created with this string as its content. thenarg.script
will be replaced with this file's name. after that, memray runs as before.memray run -c <my_file>.py
-> parser.error with help message to remove-c
option(EDIT: no longer creating a file)