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

Hybrid behavior of NG APIs #676

Closed
hynek opened this issue Aug 23, 2020 · 3 comments
Closed

Hybrid behavior of NG APIs #676

hynek opened this issue Aug 23, 2020 · 3 comments
Labels
Thinking Needs more braining.

Comments

@hynek
Copy link
Member

hynek commented Aug 23, 2020

Originally posted by @euresti in #668 (comment)

I'm working on the mypy plugin but I am gonna need to know the final answer on how "hybrid" mode wants to work if there are both annotations and attribs. I think the options are:

  1. Take only the ones that have = attr.ib()
  2. Take only the annotated ones.
  3. Take both.
  4. Raise an exception.

Hmm. I thought #3 would be easy to implement (just don't raise the error) but it turns out we can't tell the order between annotated and un-annotated attributes.

class A:
   x: int
   y = attr.ib()

cls.__annotations__  ->   [x]
cls.__dict__.keys(). -> [y]

But which is actually first?

#2 is kind of a weird one. Why throw away those attr.ib()s?
#1 is relatively easy to implement. Because it's just a try catch.
#4 Might require the user to add ClassVar in order to add it.

Oh I should note that I think this should still be fine:

@define
class A:
   x: int = field()
   y = field()

We shouldn't force you to have to annotate everything.
Hmm. That might make 4 harder to implement. :)

@hynek
Copy link
Member Author

hynek commented Aug 23, 2020

Given your explanations and my older experiments I think only 1 and 4 are feasible.

But given your example, 4 seems also more trouble and edge cases than necessary?

I kinda still feel that the sharp edges of the hybrid approach are the best trade off because it's almost impossible to get a silent failure in a real-world project. Sure maybe you get confused the first time you get missing attributes but is it worth to force people pass arguments everywhere for it? 🤔 (skip time 1 year ahead: Hynek hates himself for this thinking 🤪)

@wsanchez wsanchez added the Thinking Needs more braining. label Sep 1, 2020
@euresti
Copy link
Contributor

euresti commented Sep 2, 2020

FYI: I've implemented #1 for mypy. Here's the PR. It hasn't landed yet.

python/mypy#9396

@hynek
Copy link
Member Author

hynek commented Sep 5, 2020

(I'm keeping this open as long as the NG APIs are provisional, but I've mostly made up my mind. I'm happy to change it if substantial user complaints come in.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Thinking Needs more braining.
Projects
None yet
Development

No branches or pull requests

3 participants