Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

Allow a custom type comment prefix. #14

Closed
wants to merge 1 commit into from

Conversation

matthiaskramm
Copy link

Instead of hardcoding it, we expect callers to explicitly do something like

ast27.register_type_comment_prefix("# type: ")

before parsing.

This is the typed_ast counterpiece to python/mypy#2030.

Instead of hardcoding it, we expect callers to explicitly do something like
    ast27.register_type_comment_prefix("# type: ")
before parsing.
@ddfisher
Copy link
Collaborator

Thanks for the PR! Before getting down to a finer-grained review, I have a few big picture comments:

  • This library is intended to parse PEP 484-compliant typed code, so it should always parse # type: comments without any additional configuration.
  • I'd like for this library to be able to parse comments with arbitrary prefixes (when configured to do so) but I don't think they should be parsed as type comments, because -- per PEP 484 -- they aren't (and because this would severely limit what you can do with them). Instead, I'd like to see a new ast node in the style of TYPE_IGNORE (i.e. stored at a file level by line number) with the prefix and contents of the comment as fields.

@matthiaskramm
Copy link
Author

So are you saying that you want two kinds of nodes for comments, one for # type: ignore and one for free-form comments?

Or can we map the former to the latter, and supply the default configuration?

@ddfisher
Copy link
Collaborator

Yes. I think that # type: ignores and freeform comments are quite different beasts, and should therefore be different nodes.

@matthiaskramm
Copy link
Author

It feels odd to me to make # type: ignore and # mypy: ignore different nodes. Maybe the "ignore" comments should have a configurable prefix, and then on top of that we have a pattern for freeform comments?

@ddfisher
Copy link
Collaborator

ddfisher commented Sep 1, 2016

It should turn out simpler overall to just have a pattern for freeform comments. My understanding is that the PEP doesn't discuss custom type prefixes (type ignore or otherwise), so I'm reluctant to add those explicitly to typed_ast.

@ddfisher
Copy link
Collaborator

This is still likely not the approach we want. Closing due to inactivity.

@ddfisher ddfisher closed this Sep 15, 2017
tbbharaj pushed a commit to tbbharaj/typed_ast that referenced this pull request Dec 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants