Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
bpo-38307: Add end_lineno attribute to pyclbr _Objects#24348
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
bpo-38307: Add end_lineno attribute to pyclbr _Objects#24348
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…nstead to the stack
…va254084/cpython into endline-in-readmodule-module
…in the nest functions
…addi/cpython into endline-in-readmodule-module
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 revised patch. On the issue, I questioned whether we can insert end_lineno within the existing signatures. I have posted to pydev for additional opinions. The patch otherwise looks fine except for the blurb, which I would revise before merging.
| @@ -0,0 +1,4 @@ | |||
| Adds end line no in class' use: generating dependency. | |||
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.
I can’t parse this line 🙂
(and second line starting with colon seems strange)
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.
Neither could I nor the doc tests. I replaced it.
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.
Thanks,@terryjreedy !
But how could you push a change to my forked repository?
(I am not objecting to it, I just do not understand how another person can push to mine repo)
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.
When you made the PR, you left a checkmark in the box that says to allow core developers ('Members of Python' when you mouse over the icons) to modify the files in this branch. If you had removed it, I would have asked to put is back in if possible. To pull to your local repository, which you should do before making further changes, checkout this branch, endline-in-readmodule-module, and, I believe, if you defined 'origin' as the devguide suggests
git pull origin endline-in-readmodule-moduleDon't use rebase.
terryjreedy commentedJan 27, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Tests/macOS just says 'Error'. I suspect that this is not related to patch. The revised blurb should pass. It may still be too wordy. The person merging this should likely add a What's New entry. The details will depend on where the new argument is added. This is worth an addition to Misc/Acks. This should not be done until just before merging, or after. |
Since retests had the same 'default role used' complaint, I removed all markup. |
Thanks,@terryjreedy . All tests passed, does this mean the PR will be merged as per the standards? |
There is adiscussion about this change : it breaks compatibility but for something that was not supposed to be used directly — more changes are suggested to break with a clear message rather than appear to work. |
@terryjreedy , so should I modify the PR with |
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.
Before merging, an entry forpyclbr should be added to Improved Modules in Doc/whatsnew/3.10.rst. I think the following is enough.
Add an 'end_lineno' attribute to the Class and Function objects that appear in the
tree returned by pyclbr functions. (Contributed by Aviral Srivastava inbpo-38307.)
(With bpo marked-up as for other entries.)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
I am making the changes now. |
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Éric Araujo <merwok@netwok.org>
Thank you@terryjreedy ! Does this mean that new installations of Python will have this change or should one wait for the tag release? Thanks again! |
As a new feature, this will be released in Python 3.10. |
I don't know what you mean by a tag release, but 3.10.0a5, with Windows and macOS installers should be coming soon and will include this. 3.10.0 should be out next September. |
For back-compatibility, make the new constructor parameter for public classes Function and Classkeyword-only with a default of None.Co-authored-by: Aviral Srivastava <aviralsrivastava@Avirals-MacBook-Air.localCo-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Uh oh!
There was an error while loading.Please reload this page.
For back-compatibility, make the new constructor parameter for public classes Function and Class
keyword-only with a default of None.
bpo-38307: Provide class' and function's end line in order to provide the scope of the classes and functions in a module.
https://bugs.python.org/issue38307