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

Move variance docs to new page #3429

Merged
merged 5 commits into from
May 26, 2017
Merged

Conversation

quartox
Copy link
Contributor

@quartox quartox commented May 23, 2017

Variance is one of the most confusing topics for novice users. We want to link to this page directly in error messages relating to variance and put it in a single page to be easier to find.

@quartox
Copy link
Contributor Author

quartox commented May 23, 2017

@gnprice

@gnprice
Copy link
Collaborator

gnprice commented May 23, 2017

Thanks! Looking.

Would you add just a sentence or two to the description explaining the motivation for this? (That'll become the commit message when we "squash and merge" the change in.) Basically that we want a stable, short-named place to link to in error messages, like #3411 .

Also a small comment: I think the .. invariance-vs-covariance: bit can be dropped -- it existed to create an anchor for a link like #invariance-vs-covariance to point to this section in the middle of the long page.

@quartox
Copy link
Contributor Author

quartox commented May 23, 2017

I just edited the top comment with the description, is that the message that will be added when squashing? or do I need to put it somewhere else.

@quartox
Copy link
Contributor Author

quartox commented May 24, 2017

If we are fine with this I can update the other PR with the new link.

@quartox
Copy link
Contributor Author

quartox commented May 24, 2017

Or make more changes as we started discussing.

@gvanrossum
Copy link
Member

I'm confused -- I thought we were going to move the text to a new page? Did the plan change? (AFAIK Greg tried to create an alias on RTD and it didn't work.)

@quartox
Copy link
Contributor Author

quartox commented May 25, 2017

@gnprice argued for just changing the alias and restructuring the docs but we never finished the conversation. We should mark this WIP, but I am following his recommendation and don't have a clear idea for how to enhance the docs myself.

@gvanrossum
Copy link
Member

IIUC the most recent conversation I had about this (some time Monday) was that aliases don't even work and even if they did would be too fragile (and depend on RTD hosting). I even recall @gnprice leaving some comments describing the (negative) outcome of his experiments somewhere, but I cannot find them now.

@quartox
Copy link
Contributor Author

quartox commented May 25, 2017

The conversation we had was to simply make the alias shorter. I feel like variance might justify its own section since it is such a confusing topic to newcomers. @gnprice was worried about developing a pattern of adding documentation urls to error messages and ending up with too many top level urls. He was thinking of creating a single page with all of the urls in error messages and renaming/refactoring the "common problems" page to serve that purpose.

The initial part of this process was shortening the anchor for the variance discussion, but it is not a very coherent pull request as is, just a half formed idea.

@quartox
Copy link
Contributor Author

quartox commented May 25, 2017

Sadly this was all in person so it is all a second hand discussion right now.

@gvanrossum
Copy link
Member

IIUC @gnprice will be back at the sprint tomorrow to set us straight.

@gvanrossum
Copy link
Member

I talked it over with @gnprice off-line and the conclusion was that a separate variance page is not needed. (The problem with '#' in error messages was solved separately in #3417.)

However this PR still looks like it needs work, since the revision_history.rst page also references this anchor.

1 similar comment
@gvanrossum
Copy link
Member

I talked it over with @gnprice off-line and the conclusion was that a separate variance page is not needed. (The problem with '#' in error messages was solved separately in #3417.)

However this PR still looks like it needs work, since the revision_history.rst page also references this anchor.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Also change the reference in revision_history.rst.

@gvanrossum gvanrossum merged commit c0f7bba into python:master May 26, 2017
@gvanrossum
Copy link
Member

Thanks!

carljm added a commit to carljm/mypy that referenced this pull request May 30, 2017
* master: (23 commits)
  Make return type of open() more precise (python#3477)
  Add test cases that delete a file during incremental checking (python#3461)
  Parse each format-string component separately (python#3390)
  Don't warn about returning Any if it is a proper subtype of the return type (python#3473)
  Add __setattr__ support (python#3451)
  Remove bundled lib-typing (python#3337)
  Move version of extensions to post-release (python#3348)
  Fix None slice bounds with strict-optional (python#3445)
  Allow NewType subclassing NewType. (python#3465)
  Add console scripts (python#3074)
  Fix 'variance' label.
  Change label for variance section to just 'variance' (python#3429)
  Better error message for invalid package names passed to mypy (python#3447)
  Fix last character cut in html-report if file does not end with newline (python#3466)
  Print pytest output as it happens (python#3463)
  Add mypy roadmap (python#3460)
  Add flag to avoid interpreting arguments with a default of None as Optional (python#3248)
  Add type checking plugin support for functions (python#3299)
  Mismatch of inferred type and return type note (python#3428)
  Sync typeshed (python#3449)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants