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.

Sign up for GitHub

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

Jump to bottom

Added support for keyword-only arguments on Python 3+ [rebase] #411

Merged
merged 14 commits into from
Aug 11, 2018
Merged

Added support for keyword-only arguments on Python 3+ [rebase] #411

merged 14 commits into from
Aug 11, 2018

Conversation

asford
Copy link
Contributor

@asford asford commented Jul 24, 2018 •
edited
Loading

This is a rebase-and-extension of pull #281. @malinoff (& @hynek), I'm happy to handle this as a sub-pull of that request or merge independently.

Closes #335, closes #106, closes #38.

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

  • Added tests for changed code.
  • New features have been added to our Hypothesis testing strategy.
  • Changes or additions to public APIs are reflected in our type stubs (files ending in .pyi).
  • Updated documentation for changed code.
  • Documentation in .rst files is written using semantic newlines.
  • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
  • Changes (and possible deprecations) have news fragments in changelog.d.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

@asford asford changed the title Issue 106 Added support for keyword-only arguments on Python 3+ [rebase] Jul 24, 2018
@asford
Copy link
Contributor Author

asford commented Jul 24, 2018

@hynek I think this extends #281 to resolve the feedback you provided in that review. Would you mind taking a look at this implementation?

@hynek
Copy link
Member

hynek commented Jul 24, 2018

Thanks for reviving this! I’ll be at EuroPython for the rest of the week but will take a look once I have the time. You’ve Got a typo in the stubs btw: kw_onlly or something like that (on phone sorry).

Other reviewers are invited to pick up my slack of course. ;)

@asford
Copy link
Contributor Author

asford commented Jul 24, 2018

Great, thanks! Before I start covering my tracks, is --force push ok for fixups in PRs in this repo?

@hynek
Copy link
Member

hynek commented Jul 24, 2018

It’s your repo, do whatever the hell pleases you. We’ll squash at the end anyway.

asford added 2 commits July 24, 2018 10:49
@asford
Add attr.s-level kw_only flag.
8c14ff0 
Add `kw_only` flag to `attr.s` decorator, indicating that all class
attributes should be keyword-only in __init__.

Minor updates to internal interface of `Attribute` to support
evolution of attributes to `kw_only` in class factory.

Expand examples with `attr.s` level kw_only.
@asford
Add kw_only to type stubs.
69ad3aa
@asford
Update changelog for rebased PR.
117b81e 
Hear ye, hear ye. A duplicate PR is born.
@asford
Copy link
Contributor Author

asford commented Jul 24, 2018

Fixed .pyi typo and delinted; ready for review at everyone's convenience.

hynek
hynek requested changes Jul 28, 2018
View reviewed changes
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.

Overall this looks pretty good! I have a bunch of minor things and one major question. :)

docs/examples.rst Outdated
>>> A(a=1)
A(a=1)

`kw_only` may also be specified at via ``attr.s``, and will apply to all attributes:

This comment was marked as spam.

Sign in to view

docs/examples.rst Outdated



If you create an attribute with ``init=False``, ``kw_only`` argument is simply ignored.

This comment was marked as spam.

Sign in to view

docs/examples.rst Outdated
...
TypeError: B() missing 1 required keyword-only argument: 'b'

If you omit ``kw_only`` or specify ``kw_only=False``, then you'll get an error:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/attr/_make.py
@@ -206,6 +212,7 @@ def attrib(
converter=converter,
metadata=metadata,
type=type,
kw_only=kw_only,

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/attr/_make.py Outdated
@@ -736,6 +760,10 @@ def attrs(
Attributes annotated as :data:`typing.ClassVar` are **ignored**.

.. _`PEP 526`: https://www.python.org/dev/peps/pep-0526/
:param bool kw_only: Make all attributes keyword-only (Python 3+)
in the generated ``__init__`` (if ``init`` is ``False``, this
parameter is simply ignored).

This comment was marked as spam.

Sign in to view

tests/test_make.py
x = attr.ib(default=None)
y = attr.ib()

attrs, super_attrs, _ = _transform_attrs(C, None, False, True)

This comment was marked as spam.

Sign in to view

tests/test_make.py
x = attr.ib(init=False, default=0, kw_only=True)
y = attr.ib()

c = C(1)

This comment was marked as spam.

Sign in to view

tests/test_make.py
y = attr.ib()

with pytest.raises(ValueError) as e:
_transform_attrs(C, None, False, False)

This comment was marked as spam.

Sign in to view

tests/test_make.py
with pytest.raises(TypeError):
C(0, y=1)

c = C(x=0, y=1)

This comment was marked as spam.

Sign in to view

tests/test_make.py
with pytest.raises(TypeError):
C(1)

c = C(x=0, y=1)

This comment was marked as spam.

Sign in to view

@hynek hynek added this to the 18.2 milestone Jul 28, 2018
hynek
hynek requested changes Aug 1, 2018
View reviewed changes
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.

We’re getting there!

src/attr/_make.py Outdated
@@ -1558,6 +1613,17 @@ def from_counting_attr(cls, name, ca, type=None):
**inst_dict
)

# Don't use attr.evolve since fields(Attribute) doesn't work
def _evolve(self, **changes):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/attr/_make.py
if kw_only_args:
if PY2:
raise PythonTooOldError(
"Keyword-only arguments only work on Python 3 and later."

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/attr/exceptions.py

class PythonTooOldError(RuntimeError):
"""
An ``attrs`` feature requiring a more recent python version has been used.

This comment was marked as spam.

Sign in to view

tests/test_make.py
@attr.s(kw_only=True)
class ClassLevel(object):
a = attr.ib()

This comment was marked as spam.

Sign in to view

tests/test_make.py
@attr.s()
class AttrLevel(object):
a = attr.ib(kw_only=True)

This comment was marked as spam.

Sign in to view

@hynek hynek merged commit 123df67 into python-attrs:master Aug 11, 2018
@hynek
Copy link
Member

hynek commented Aug 11, 2018

Thanks everyone!

@hynek hynek mentioned this pull request Aug 11, 2018
Closed
6 tasks
@wbolster
Copy link
Member

this is awesome! thanks @asford for pushing this through!

@benbovy benbovy mentioned this pull request Aug 29, 2018
Closed
@dependencies dependencies bot mentioned this pull request Sep 2, 2018
Closed
@bar bar mentioned this pull request Apr 8, 2019
Closed
@gtsambos gtsambos mentioned this pull request Dec 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hynek hynek hynek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
18.2
4 participants
@asford @hynek @wbolster @malinoff