-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Improve approx scalar repr method for better readability #12665
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
Improve approx scalar repr method for better readability #12665
Conversation
|
@Zac-HD please review and let me know if there are any changes required. |
Pierre-Sassoulas
left a comment
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.
Thank you for the pull request !
Did you see this comment in the original issue : #6985 (comment) ?
|
@Pierre-Sassoulas thank you for the feedback. I had missed the comment so I have added the 'n' format specifier as well. Please let me know what else needs to changed |
Pierre-Sassoulas
left a comment
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.
Some tests results to upgrade otherwise LGTM
|
added test cases as well |
Pierre-Sassoulas
left a comment
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.
The pipeline is still failing, some existing tests need to be upgraded too.
|
@Pierre-Sassoulas updated existing tests |
Pierre-Sassoulas
left a comment
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.
Great, do you mind adding a changelog too, please ?
src/_pytest/runner.py
Outdated
| sys.last_value = e | ||
| if sys.version_info >= (3, 12, 0): | ||
| sys.last_exc = e | ||
| sys.last_exc = e # type: ignore[attr-defined] |
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.
Seems like this type ignores are unused in the CI (see pre-commit job).
nicoddemus
left a comment
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, left some minor comments.
Co-authored-by: Bruno Oliveira <[email protected]>
Co-authored-by: Bruno Oliveira <[email protected]>
Co-authored-by: Bruno Oliveira <[email protected]>
|
@nicoddemus thanks for the suggestions. can you let me know what could be causing the readthedocs check to fail. I don't understand the error |
|
Looks like an issue unrelated to your PR: sphinx-contrib/sphinxcontrib-towncrier#92 |
It might be some regression on towncrier/sphinx extension, I noticed there was a new release towncrier release 5 hrs ago: Don't worry, seems unrelated to your changes. |
The-Compiler
left a comment
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.
Looking great!
|
Opened #12675 to fix the readthedocs/towncrier issue, we can update with |
Co-authored-by: Pierre Sassoulas <[email protected]>
Co-authored-by: Pierre Sassoulas <[email protected]>
Co-authored-by: Pierre Sassoulas <[email protected]>
|
Great ! |
Closes #6985
closes #XYZWto the PR description and/or commits (whereXYZWis the issue number).changelogfolder, with a name like<ISSUE NUMBER>.<TYPE>.rst.AUTHORSin alphabetical order.