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

Erroneous duplicate argument error #244

Open
ericfrederich opened this issue Sep 8, 2017 · 4 comments
Open

Erroneous duplicate argument error #244

ericfrederich opened this issue Sep 8, 2017 · 4 comments
Labels

Comments

@ericfrederich
Copy link

this line makes it impossible to have a class like the following. I believe it should be possible.

@attr.s
class Foo:
    spam = attr.ib()
    eggs = attr.ib()
    _eggs = attr.ib()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/eric/.virtualenvs/example/lib/python3.5/site-packages/attr/_make.py", line 391, in attributes
    return wrap(maybe_cls)
  File "/home/eric/.virtualenvs/example/lib/python3.5/site-packages/attr/_make.py", line 364, in wrap
    cls = _add_init(cls, effectively_frozen)
  File "/home/eric/.virtualenvs/example/lib/python3.5/site-packages/attr/_make.py", line 569, in _add_init
    bytecode = compile(script, unique_filename, "exec")
  File "<attrs generated init 2b427e104daf8a0e2c385316a27a312cdcc1c6d1>", line 1
SyntaxError: duplicate argument 'eggs' in function definition
@RonnyPfannschmidt
Copy link

i do wonder if there is ever an actual case where such a attrbute naming adds value,
from my pov its quite reckless to have those as you show, i believe there should only be better error message

@ericfrederich
Copy link
Author

ericfrederich commented Sep 8, 2017

I'll write up my use-case after lunch. But for now I can say it feels a bit weird having the __init__ names to be different from the class attribute names.

>>> import attr
>>> 
>>> 
>>> @attr.s
... class Foo:
...   spam = attr.ib()
...   _eggs = attr.ib()
... 
>>> 
>>> 
>>> Foo(spam='a', _eggs='b')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init__() got an unexpected keyword argument '_eggs'
>>> Foo(spam='a', eggs='b')
Foo(spam='a', _eggs='b')

@ericfrederich
Copy link
Author

Okay... here us my use case (perhaps justified, perhaps not).

I'm extracting GitLab information using python-gitlab.
In the end I need to serialize it to json in this format to import into JIRA.
The basic idea is that I have a single attr.s object which I can serialize directly into the format needed by JIRA.

To accomplish this, I have two different kinds of attributes. Ones that will be serialized, and ones that won't. They are differentiated by whether they have a leading underscore or not.

# I serialize it like this...
d = attr.asdict(extract, filter=lambda a, v: not a.startswith('_'))
json.dump(d, fp, indent=2)  # produces exactly what will be fed into JIRA

So... that is the need for the underscore (to mark whether to serialize it or not).

Now you may be wondering about why have two fields with the same name?

Well, these attr.s objects are basically a 1:1 mapping to the underlying python-gitlab objects. Unfortunately the python-gitlab objects aren't as nice as attr.s classes... two objects representing the same thing don't equate... user1 == user2 will always be False. Because of this, I like to keep the original python-gitlab objects around. For example:

@attr.s
class Comment:
    author = attr.ib()
    message = attr.ib()

@attr.s
class Issue:
    comments = attr.ib()  # list of the above attr.s Comment objects
    _comments = attr.ib()  # list of underlying GitLab Comment objects

@wsanchez
Copy link

YYMV, but I think it's a bad pattern to tightly couple the names of symbols in a class def to your serialization format. This probably won't be the only time that this apparent simplicity bites you.

In this case, you are competing with attrs over what the leading underbar convention means. I guess the most flexible thing attrs can do is give attr.ib yet another arg, for naming the argument to __init__, but it seems a lot simpler to just rename _comments in the above example.

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

No branches or pull requests

3 participants