Skip to content

Switch to NG APIs in docs #863

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 17 commits into from
Nov 22, 2021
Merged

Switch to NG APIs in docs #863

merged 17 commits into from
Nov 22, 2021

Conversation

Tinche
Copy link
Member

@Tinche Tinche commented Nov 11, 2021

Here's what I'm thinking.

Switch @attr.s to a bare @define. This includes also doing from attr import define. I consider @define to be as cleanest as we can get, and I love it.

Remove inheriting from object pretty much everywhere. Use modern type annotations whereever possible (so list[int] instead of List[int]). Essentially use all the affordances of modern Python to make the examples as clean as possible.

Avoid = field()/= attr.ib() whereever possible, while still leaving it in a few places. Again, the syntax with just the type annotation is extremely clean and will probably help sell this to new folks. Prefer field to attr.ib.

I haven't gone through all the files yet, this is just the start.

@Tinche
Copy link
Member Author

Tinche commented Nov 12, 2021

Did some more converting. A good next step would be to say that type annotations are completely optional, can be replaced by field or annotating with Any, and we don't do anything with them by default. I would also add that they are very recommended: mypy supports them, and even if a person is not using mypy, they still serve as documentation, can have uses to companion libraries and help with editor autocomplete.

@hynek
Copy link
Member

hynek commented Nov 12, 2021

Did some more converting. A good next step would be to say that type annotations are completely optional, can be replaced by field or annotating with Any, and we don't do anything with them by default. I would also add that they are very recommended: mypy supports them, and even if a person is not using mypy, they still serve as documentation, can have uses to companion libraries and help with editor autocomplete.

I would push back a little on the last part: unchecked type annotations are potentially worse than no type annotations. The number of packages I had to block in mypy.ini is growing every day and even in applications an unchecked type annotation is akin to a comment: it means nothing and can be even confusing.

The first part we could probably put into types.rst.

We need to write a chapter on the names tho, but that's something I can do when you're done or even in parallel. Effectively explain the various types and some historic context. Closing #726 in the process.

P.S. CI is failing 😇

@Tinche
Copy link
Member Author

Tinche commented Nov 12, 2021

I would push back a little on the last part: unchecked type annotations are potentially worse than no type annotations. The number of packages I had to block in mypy.ini is growing every day and even in applications an unchecked type annotation is akin to a comment: it means nothing and can be even confusing.

Interesting. We had been using type annotations for about 3 years without ever using Mypy, and I don't mean only in classes; function arguments too. We found it better than nothing, and would use the editor hover popup as a poor man's Mypy :)) But I appreciate your viewpoint too. We are not a library, but an end consumer of them.

@Tinche
Copy link
Member Author

Tinche commented Nov 17, 2021

How do I fix init.rst:196:more than one target found for 'any' cross-reference 'frozen': could be :py:func:`attr.setters.frozen` or :py:func:`attr.frozen?

Copy link
Member

@hynek hynek left a comment

Choose a reason for hiding this comment

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

  • Hashing could use some attr.s replacements in copy.
  • some smaller stuff
  • otherwise promising

thanks for doing god's work here Tin!

@Tinche Tinche requested a review from hynek November 22, 2021 00:38
Copy link
Member

@hynek hynek left a comment

Choose a reason for hiding this comment

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

Welcome to the future zone!

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