Skip to content

bpo-39199: Add descriptions of non-deprecated AST nodes to the AST module documentation #17812

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

Merged
merged 6 commits into from
Mar 2, 2020

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Jan 3, 2020

@pablogsal pablogsal force-pushed the better_ast_docs branch 6 times, most recently from 7d5f35d to 7c3e06a Compare January 3, 2020 14:55
@pablogsal pablogsal changed the title bpo-39199: Add non-deprecated nodes to the AST documentation bpo-39199: Add descriptions of non-deprecated AST nodes to the AST module documentation Jan 3, 2020
@pablogsal pablogsal added the docs Documentation in the Doc dir label Jan 3, 2020
@pablogsal pablogsal force-pushed the better_ast_docs branch 5 times, most recently from 4f16572 to 5095d7b Compare January 3, 2020 23:01
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 3, 2020
@pablogsal pablogsal removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 3, 2020
@willingc
Copy link
Contributor

willingc commented Jan 6, 2020

@pablogsal Just getting back from vacation. Will look at this in the next day or so. Happy New Year too!

Co-Authored-By: Karthikeyan Singaravelan <[email protected]>
Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pablogsal This is wonderful. Great job. A few little nits but this looks ready to merge. Thanks!

@JulienPalard
Copy link
Member

LGTM, @pablogsal take a look at @willingc comments and it's good for me.

@takluyver
Copy link
Contributor

I was slightly surprised when I consulted the AST module docs today to see that a large part of Green Tree Snakes has been incorporated with no visible attribution (in the docs themselves - I can see it's acknowledged in this PR).

I am pleased to see it there - I think this should have been part of the official AST docs in the first place, and I'm sure it will be kept up to date better here than as a random separate project. But at the same time, a fair amount of effort (mostly other contributors' in the last few years) went into creating and curating those descriptions. I would have expected someone to ask before republishing it.

I never actually got round to adding any kind of license to the GTS repo, though that's due to oversight & laziness rather than any objection to the idea. If I had, it would likely be something like CC-BY or BSD, though I'd be happy to waive the attribution requirements (as much as I can - other contributors retain their rights) if that was even a small obstacle to improving the Python docs.

If you think it's appropriate, I would be delighted to see a little note somewhere by this section mentioning something like "these descriptions of node classes were adapted from Green Tree Snakes". I think that's the easiest way to acknowledge something that many people have worked on. If there's no natural way to do this visibly in the documentation, I'd settle for the compromise of putting it in a comment in the rst source.

@pablogsal
Copy link
Member Author

Thanks a lot for reaching out @takluyver !

If you think it's appropriate, I would be delighted to see a little note somewhere by this section mentioning something like "these descriptions of node classes were adapted from Green Tree Snakes". I think that's the easiest way to acknowledge something that many people have worked on. If there's no natural way to do this visibly in the documentation, I'd settle for the compromise of putting it in a comment in the rst source.

Absolutely, I think it makes sense. Let's add some visible attribution on the documentation itself! Would you mind reviewing the PR once I make it?

@takluyver
Copy link
Contributor

Thanks! I'd be happy to review a PR.

@pablogsal
Copy link
Member Author

Thanks! I'd be happy to review a PR.

Opened #24727

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants