Skip to content
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

Explain types and how they work better #3892

Closed
1 of 2 tasks
rchiodo opened this issue Jan 31, 2023 · 11 comments
Closed
1 of 2 tasks

Explain types and how they work better #3892

rchiodo opened this issue Jan 31, 2023 · 11 comments
Labels
enhancement New feature or request epic

Comments

@rchiodo
Copy link
Contributor

rchiodo commented Jan 31, 2023

Linked items:

See the discussion here:
microsoft/pyright#3953 (reply in thread)

And the numerable other cases where how types should be used was confusing.

A lot of the ones in this category might be avoided if we could explain things more clearly:
https://github.com/microsoft/pyright/issues?q=is%3Aissue+is%3Aclosed+label%3A%22as+designed%22

This issue is to provide a way to better explain python typing to Pylance users.

@rchiodo rchiodo added the enhancement New feature or request label Jan 31, 2023
@rchiodo
Copy link
Contributor Author

rchiodo commented Jan 31, 2023

Potential idea:
Every pylance error should explain why it happened and give examples on how to use the typing construct.

@rchiodo
Copy link
Contributor Author

rchiodo commented Jan 31, 2023

Another idea:
Train ChatGPT on Eric's responses.

@erictraut
Copy link
Contributor

Most of pyright's errors do attempt to explain the cause of an error, but there are some (think "syntax error") where it should be self-explanatory. In other cases (e.g. "TypeVar appears only once in signature" or "expression is not used") where the diagnostic is indicative of some fundamental misunderstanding on the part of a user, but it's difficult to explain succinctly what the source of that confusion is or how they might remedy the error.

Other investment ideas:

  • More complete & better documentation
  • Tutorials (articles, blogs, videos)

@rchiodo
Copy link
Contributor Author

rchiodo commented Jan 31, 2023

This is what ChatGPT3 said about the issue mentioned:

image

@erictraut
Copy link
Contributor

That's a pretty good explanation, but the recommended fix is likely wrong. (I say "likely wrong" beacuse it's difficult to say what the user actually intended in this case.) A better recommendation in this case is "change the annotation for parameter xynum to XY[int, int]". Or "Change the definition of XYNUM to a type alias: XYNUM: TypeAlias = XY[int, int]".

@rchiodo
Copy link
Contributor Author

rchiodo commented Jan 31, 2023

Well, this is why it needs to be trained only on your responses :)

@rchiodo
Copy link
Contributor Author

rchiodo commented Feb 16, 2023

Another idea:

  • Give each error message a link
  • Link points to documentation for that specific error in pyright docs
  • Docs describe the error message in more detail, give links to peps, give examples of when it might happen and how you fix it

Here's an example:

            this._addDiagnostic(
                this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
                DiagnosticRule.reportGeneralTypeIssues,
                Localizer.Diagnostic.annotationNotSupported(),
                node.chainedTypeAnnotationComment
            );

That would instead put up a message like so:

Type annotation not supported. [More Info](https://github.com/microsoft/pyright/docs/errors/typeannotationnotsupported.md#chained)

That would jump to

  • A doc specifically for Type annotation not supported
  • A subsection with an example of with a chained annotation, explaining that the second annotation should be removed

@rchiodo
Copy link
Contributor Author

rchiodo commented Feb 16, 2023

@heejaechang had another idea, give each diagnostic a unique ID so that users can actually search for these errors.

That would change the above to be something like:

Type annotation not supported. [More Info](https://github.com/microsoft/pyright/docs/errors/1110.md#chained)

@erictraut
Copy link
Contributor

Creating a unique ID and documentation for every diagnostic message would be a very significant undertaking and would come with significant maintenance cost. Last time I counted, there were over 600 diagnostic messages. For those reasons, I would vote against that approach. I think some of the other ideas above are much more feasible.

@rchiodo
Copy link
Contributor Author

rchiodo commented Feb 21, 2023

Had a meeting with the typescript team. Here's a summary of what was discussed:

  • Documenting every message was seen as not useful. Most messages are self explanatory and the one's that weren't were often very hard to have a complete set of examples. In some cases, SO questions could be generated/seeded with a specific error message
  • Error ids were important for finding things for users (and not error names). Especially in other languages other than English
    • Error ids strings being unique was also super helpful. Meaning TS11111 as opposed to just 11111. It lets that error be unambiguous when customers looked for it.
  • Error ids were tracked in a big json file. Usually groups of similar diagnostics were setup in the same range. Maintaining this list was not seen as a problem.

@rchiodo
Copy link
Contributor Author

rchiodo commented Apr 20, 2023

C# team had similar feedback:

  • First message needed to be better. That was their biggest gripe. Their initial message was often too obtuse.
  • Searching for message ids was crucial
  • Online help (on msdn) was less important. Still useful, but not as much as getting the initial message right.

@luabud luabud added the epic label May 8, 2023
@rchiodo rchiodo closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request epic
Projects
None yet
Development

No branches or pull requests

3 participants