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
Added programmatic pip list #864
base: new-apis_v0.1.0-dev
Are you sure you want to change the base?
Added programmatic pip list #864
Conversation
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## new-apis_v0.1.0-dev #864 +/- ##
=======================================================
- Coverage 94.41% 94.28% -0.14%
=======================================================
Files 158 158
Lines 9329 9339 +10
=======================================================
- Hits 8808 8805 -3
- Misses 521 534 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM
Mmmm. One more thought. |
This is a good point. Should we flush the output to a file, |
How do we use it? If we really need list of packages I'd prefer to use stdout & bash pipes instead of hardcoded flushing to file, so user do this:
So user can both (1) output debugInfo to the terminal to see what's happening, and (2) store it to the file for easier usage if needed |
That would be totally fine for a user who is conversant with a terminal interface, but for a "regular" user, the file creation might work better, no? Something like this: python gandlf debug-info
Debug information has been written to "XYZ/gandlf_debug.info"; please either copy and paste this file or its contents to further guide GaNDLF maintainers towards solving your issue. As you know, I have a personal preference for using Thoughts? |
Yeah, that's a good solution if we need pip versions. |
Yup, adding a |
Just to highlight: all solutions look quite similar to me, I don't have a strong preference of one of them:) So I'm just asking and offering alternatives. Any usable solution is ok for me |
Done! |
Hey @VukW, could you please help me with this error? I am guessing it is coming from the entrypoint test, but I am unable to figure out what to change... ....
FAILED testing/entrypoints/test_debug_info.py::test_case[case0] - AssertionError: Function was not called with the expected arguments: expected_args={'verbose': True} vs actual_args={'verbose': False}, diff ['verbose']
FAILED testing/entrypoints/test_entrypoints_existence.py::test_command_execution[gandlf_debugInfo --help] - subprocess.CalledProcessError: Command '['gandlf_debugInfo', '--help']' returned non-zero exit status 1. |
@scap3yvt Hi man!
so whenever you run
That test means that we can run the following commands:
and all of them should lead to executing python
And in this case we should expect
|
Thanks a ton for the guidance, @VukW! I updated the code appropriately. |
This (or an extended example set) would be an awesome addition for the migration guide (#843)! 😄 |
Hey @VukW, can you help with the errors I am seeing with the entrypoints [ref]? FAILED testing/entrypoints/test_debug_info.py::test_case[case0] - TypeError: __init__() got an unexpected keyword argument 'metavar'
FAILED testing/entrypoints/test_debug_info.py::test_case[case1] - TypeError: __init__() got an unexpected keyword argument 'metavar'
FAILED testing/entrypoints/test_entrypoints_existence.py::test_command_execution[gandlf_debugInfo --help] - subprocess.CalledProcessError: Command '['gandlf_debugInfo', '--help']' returned non-zero exit status 1. |
Hey @VukW, do you think you could shed some light on the error I am seeing? I am pretty sure it is related to this portion of the test: CliCase(
should_succeed=True,
new_way_lines=["-v"],
old_way_lines=["-v True"], # <<<<< here
expected_args={"verbose": True},
), Which results in the following error message: 2024-05-04T13:46:01.2015561Z self = ArgumentParser(prog='GANDLF_DebugInfo', usage=None, description='Generate debugging information for maintainers.\n\nCo.../s44172-023-00066-3', formatter_class=<class 'argparse.RawTextHelpFormatter'>, conflict_handler='error', add_help=True)
2024-05-04T13:46:01.2018136Z status = 2, message = 'GANDLF_DebugInfo: error: unrecognized arguments: True\n' But I am unable to figure out what I am doing wrong. |
Hi @scap3yvt ,
İt is the same as you are running manually the following command:
So you can run it to see / debug what happens. Anyway, if you are changing anything it worth to run the according command manually to check if it is successful - or debug any issues if they are, it's much quicker then committing and waiting till ci step is finished:) In this specific case we expect old commands arg to be a normal flag, so you don't need to pass any value to it ( |
Thanks for your input, @VukW! The tests are passing, but the added lines are not getting covered by the tests: Any idea why this might be happening? |
@scap3yvt This happens when the section of the code is never reached. Enable verbose in tests to reach this part of the code. |
I was assuming that these lines would do the trick but they apparently don't work as expected. Any insight, @VukW? |
Hey folks, let me look in more detail a bit later cos I didn't find answer from the first glance. Actually the |
@VukW, is it possible that the |
What |
Thank you for that detailed explanation! |
Fixes #863
Proposed Changes
pip list
(as opposed to submodule call, which is unsafe)Checklist
CONTRIBUTING
guide has been followed.typing
is used to provide type hints, including and not limited to usingOptional
if a variable has a pre-defined value).pip install
step is needed for PR to be functional), please ensure it is reflected in all the files that control the CI, namely: python-test.yml, and all docker files [1,2,3].