-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
REGR: show_versions #32041
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
REGR: show_versions #32041
Conversation
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
merits test? |
@jbrockmendel looks like the fakemodule tests don't clean-up. will look tomorrow if can't get to machine later this evening. could maybe use monkeypatch VERSIONS or just delete fakemodule from VERSIONS after test |
ok, test here isnt important enough to be a blocker |
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,
Just an idea, we could rename show_versions
to _get_versions
and make it a generator yielding the version information. Then implement a small show_versions
function that prints the results of _get_versions
.
This way your tests would be much simpler and clearer, and I guess it can be useful in some cases to access the versions from Python, not just getting them printed.
Happy to get this merged as is.
have started #32042 to clean-up since |
Thanks @simonjayhawkins |
regression in #31660 (no need to backport)
master
this PR