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

bpo-33961: Adjusted docs to correct exceptions raised. #7917

Merged
merged 1 commit into from
Jul 2, 2018

Conversation

chmarr
Copy link

@chmarr chmarr commented Jun 25, 2018

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

When your account is ready, please add a comment in this pull request
and a Python core developer will remove the CLA not signed label
to make the bot check again.

Thanks again for your contribution, we look forward to reviewing it!

@chmarr
Copy link
Author

chmarr commented Jun 26, 2018

CLA has been signed.

Copy link
Member

@ericvsmith ericvsmith left a comment

Choose a reason for hiding this comment

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

Thanks!

@miss-islington
Copy link
Contributor

Thanks @chmarr for the PR, and @ericvsmith for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @chmarr and @ericvsmith, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 24d74bd8377d38528566437e70fcd72229695ac7 3.7

@serhiy-storchaka serhiy-storchaka added the docs Documentation in the Doc dir label Oct 31, 2018
@callmesangio
Copy link
Contributor

Hi @chmarr @ericvsmith,

I was looking at the dataclasses docs and it seems to me that this PR has been merged into 3.7 only, but should be backported to 3.8 and to master.
Is this correct?

Thanks,
Fabio

@ericvsmith
Copy link
Member

Just looking at the history of this PR, it looks like the change was made to 3.8 (so presumably is also in 3.9), but wasn't backported to 3.7. I haven't had time yet to verify that. But if you verify it, and you can check that 3.7 works the same as 3.8 with regard to this exception, then we should create a PR that updates 3.7.

@callmesangio
Copy link
Contributor

Hi,

I've found the behavior to be consistent across 3.7 and 3.8 (I haven't tried with 3.9):

from dataclasses import dataclass
from unittest import TestCase


class OrderedDataclassTestCase(TestCase):

    def test_lt(self):
        with self.assertRaises(TypeError):
            @dataclass(order=True)
            class C:
                def __lt__(self, other):
                    return self

    def test_le(self):
        with self.assertRaises(TypeError):
            @dataclass(order=True)
            class C:
                def __le__(self, other):
                    return self

    def test_gt(self):
        with self.assertRaises(TypeError):
            @dataclass(order=True)
            class C:
                def __gt__(self, other):
                    return self

    def test_ge(self):
        with self.assertRaises(TypeError):
            @dataclass(order=True)
            class C:
                def __ge__(self, other):
                    return self

As far as this PR is concerned, I don't find any indication about it originally targeting 3.8 🤔...I only see it's been merged into 3.7. Plus, it seems to me that the docs currently reflect this state of things:

https://docs.python.org/3.7/library/dataclasses.html#module-level-decorators-classes-and-functions (correct: TypeError)

https://docs.python.org/3.8/library/dataclasses.html#module-level-decorators-classes-and-functions (wrong: ValueError instead of TypeError)

https://docs.python.org/3.9/library/dataclasses.html#module-level-decorators-classes-and-functions (wrong: ValueError instead of TypeError)

Thanks for your time, I hope I haven't missed something obvious.

@ned-deily
Copy link
Member

Just looking at the history of this PR, it looks like the change was made to 3.8

Hmm, but the PR was, in fact, merged on the 3.7 branch so the 3.7 backport label was useless and that's why it's still there. I guess we didn't notice that the original PR was submitted against 3.7 rather than master. So it may need to be forward ported to master and 3.8.

@callmesangio
Copy link
Contributor

@ned-deily yes, that's the conclusion I've come to as well. I'm willing to open any PRs to fix this if you want; it would be my first contribution to CPython though, and I'm not sure if there are any other standard workflows that better address cases like this.

@ericvsmith
Copy link
Member

I guess I didn't notice that this was against 3.7: no wonder it wouldn't backport! @sanjioh : I think you can just make a new PR for the original issue, against master.

@callmesangio
Copy link
Contributor

@ericvsmith Done, see #17677

miss-islington added a commit that referenced this pull request Dec 25, 2019
…H-7917) (GH-17677)

(cherry picked from commit e28aff5)

Co-authored-by: Fabio Sangiovanni <4040184+sanjioh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants