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

Improving the error message when subclassing NewType #90328

Closed
Gobot1234 mannequin opened this issue Dec 24, 2021 · 9 comments
Closed

Improving the error message when subclassing NewType #90328

Gobot1234 mannequin opened this issue Dec 24, 2021 · 9 comments
Labels
3.10 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@Gobot1234
Copy link
Mannequin

Gobot1234 mannequin commented Dec 24, 2021

BPO 46170
Nosy @gvanrossum, @JelleZijlstra, @sobolevn, @Gobot1234, @Fidget-Spinner, @AlexWaygood
PRs
  • bpo-46170: Improve the error message when subclassing NewType #30268
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2022-03-08.03:51:06.533>
    created_at = <Date 2021-12-24.00:01:32.462>
    labels = ['type-feature', 'library', '3.10']
    title = 'Improving the error message when subclassing NewType'
    updated_at = <Date 2022-03-08.03:51:06.532>
    user = 'https://github.com/Gobot1234'

    bugs.python.org fields:

    activity = <Date 2022-03-08.03:51:06.532>
    actor = 'JelleZijlstra'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-03-08.03:51:06.533>
    closer = 'JelleZijlstra'
    components = ['Library (Lib)']
    creation = <Date 2021-12-24.00:01:32.462>
    creator = 'Gobot1234'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46170
    keywords = ['patch']
    message_count = 9.0
    messages = ['409114', '409181', '409184', '409203', '409209', '409213', '409214', '409215', '414713']
    nosy_count = 6.0
    nosy_names = ['gvanrossum', 'JelleZijlstra', 'sobolevn', 'Gobot1234', 'kj', 'AlexWaygood']
    pr_nums = ['30268']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue46170'
    versions = ['Python 3.10']

    @Gobot1234
    Copy link
    Mannequin Author

    Gobot1234 mannequin commented Dec 24, 2021

    I'd like to propose making the error message when subclassing typing.NewType much more understandable. Currently it looks like:

    TypeError: NewType.__init__() takes 3 positional arguments but 4 were given
    

    But I think we could do much better by adding mro_entries to the class and then just having that raise a TypeError telling users they cannot subclass NewType and they are probably looking for NewType('Subclass', OlderType). I'd be happy to patch this myself if this sounds like a good idea.

    @Gobot1234 Gobot1234 mannequin added 3.10 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Dec 24, 2021
    @sobolevn
    Copy link
    Member

    This seems like a good idea to me.

    I will send a PR for others to judge :)

    @sobolevn
    Copy link
    Member

    I'd be happy to patch this myself if this sounds like a good idea.

    Oh, please, go ahead! :)
    Would be happy to review.

    @gvanrossum
    Copy link
    Member

    Do you have evidence that people make this particular mistake often? And that the suggested solution is indeed what they should use? Why would you want to subclass UserId?

    Do we have examples of other error messages split across two lines like this?

    @Gobot1234
    Copy link
    Mannequin Author

    Gobot1234 mannequin commented Dec 26, 2021

    Do you have evidence that people make this particular mistake often? And that the suggested solution is indeed what they should use? Why would you want to subclass UserId?

    I was just trying to implement something that I thought had a non-obvious error message and isn't necessarily initially obvious if you have the idea that NewType creates a new type/class. The documentation suggests this is what you should be doing instead of subclassing it
    342e800#diff-8a0f115fde6769c122b771b6d0eca184c4580f7b5fabe2f0b0579c679424364fR101-R113

    Do we have examples of other error messages split across two lines like this?

    No if you want me to remove it thats fine by me.

    @gvanrossum
    Copy link
    Member

    Weird, the docs you cite claims

       # Also does not typecheck
       ProUserId = NewType('ProUserId', UserId)

    but when I try it, it works at runtime and passes mypy (even with --strict) and also pyright.

    So maybe those docs are incorrect? I can't find anything in PEP-484 about this.

    In any case I'd drop the newline in the message -- other "Did you mean" errors don't have those.

    @sobolevn
    Copy link
    Member

    So maybe those docs are incorrect? I can't find anything in PEP-484 about this.

    Mypy even has an explicit test that NewType of NewType should work: https://github.com/python/mypy/blob/f96446ce2f99a86210b21d39c7aec4b13a8bfc66/test-data/unit/check-newtype.test#L162-L165

    I tend to agree with this behavior. This allows us to create complex type-safe DSLs:

    from typing import NewType
    
    UserId = NewType('UserId', int)
    ProUserId = NewType('ProUserId', UserId)
    
    x: ProUserId = ProUserId(UserId(1))

    Mypy is happy with that!

    I will open a new issue about incorrect docs.

    @sobolevn
    Copy link
    Member

    Turns out modern docs already have this problem fixed: https://docs.python.org/3/library/typing.html#newtype

    @JelleZijlstra
    Copy link
    Member

    New changeset f391f9b by James Hilton-Balfe in branch 'main':
    bpo-46170: Improve the error message when subclassing NewType (GH-30268)
    f391f9b

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants