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

User-supplied metadata for attr.ib #63

Closed
wearpants opened this issue Aug 17, 2016 · 29 comments
Closed

User-supplied metadata for attr.ib #63

wearpants opened this issue Aug 17, 2016 · 29 comments
Labels

Comments

@wearpants
Copy link

wearpants commented Aug 17, 2016

It might be desirable to allow user-supplied metadata dict as part of an Attribute for use later during introspection. Something like:

@attr.s
class A:

    boring = attr.ib()
    i_am_special = attr.ib(metadata = {'special': True})

    def do_things(self):
        for a in attr.fields(type(self)):
            if a.metadata.get('special', False):
                self.do_something(a.name)
            else:
                self.do_something_else(b.name)

(above example is assuming attrs populates metadata with an empty dict if not provided.

Perhaps this is encourages more metaprogramming than is healthy, but I had a need for this in my first use of attrs; the alternative was keeping an explicit list of special attribute names, which is not very DRY. Just looking for design/desirability feedback before working on a patch.

@Tinche
Copy link
Member

Tinche commented Aug 18, 2016

This might be an interesting feature to somehow support. I myself am working on a library that would benefit from metadata like this (types in my case).

First of all, there are some technical issues, in that Attribute instances are immutable and Python doesn't have a frozendict, so we'd probably have to do this differently (like a tuple of tuples, or just put the metadata somewhere else).

Secondly, you can already do this, but it's not that straightforward. You'd have to define your own subclass of _CountingAttr to receive the metadata in the constructor, and your own decorator to get your metadata out of your custom _CountingAttrs and put it somewhere else before @attr.s switches _CountingAttrs to Attributes.

Also take a look at http://attrs.readthedocs.io/en/stable/extending.html.

@hynek
Copy link
Member

hynek commented Aug 18, 2016

I think it would be very attrs to expect metadata to be a proper class. :)


Since I want attrs to be extensible, I’m rather positively inclined to such an feature.

@wearpants
Copy link
Author

Yeah, was thinking about how attr might play with type annotations...

Re: immutability - maybe metadata could be excluded from Attribute's hash/cmp, with a warning to users that if they modify it, they've made their bed & must lie in it. Alternately storing metadata as 2-tuples with an accessor that builds the dict seems possible.

Another option if true immutability is really needed for Attributes would be a __attrs_metadata__ dict on the class with name => metadata and a helper method/property on Attribute to retrieve it.

But basically, having some way to associate arbitrary info that attracts itself doesn't use/care about seems useful.

@wearpants
Copy link
Author

I think it would be very attrs to expect metadata to be a proper class. :)

Hmm, going the other direction, why require metadata to be any particular type in the first place? Could just be a slot for any user supplied object, without any implied semantics, kinda like how annotations (used to) work.

@wearpants
Copy link
Author

wearpants commented Aug 18, 2016

Probably deserves it own issue, but this is broadly similar to supporting PEP 526-style first class attribute annotations. See also this section which hint at a subset of attrs functionality

@hynek hynek added the Feature label Aug 21, 2016
@pgruenbacher
Copy link

What I want for my personal use is something like go struct tags.http://stackoverflow.com/questions/10858787/what-are-the-uses-for-tags-in-go

@attr.s()
class C(object):
     first_name = attr.ib() + 'json:"firstName"'
     last_name = attr.ib(tag='json:"lastName" sql:"lastname"')

I'll fork and try how I like something like this for my own use. I think the first class attribute annotations solve this for me as well, but I need a placeholder for now and the + 'tag' may temporarily hold me over

@wearpants
Copy link
Author

The second style is covered by my metadata approach - a single additional
field on attributes with a dict (or any user-supplied object) that you can
interpret however you want.

The first style (+) is just gross.

Attribute annotations certainly complement this, and I'd like to see attr
support them by dynamic introspection (ie, Attribute.metadata is sugar
for reading the annotation off the class). I suppose you could argue that
there are use cases where you want independent metadata and annotation (for
making an attr class work with some 3rd party library that has its own
uses/opinions for the annotations).

On Aug 30, 2016 10:59 AM, "Paul Gruenbacher" notifications@github.com
wrote:

What I want for my personal use is something like go struct tags.
http://stackoverflow.com/questions/10858787/what-are-
the-uses-for-tags-in-go

@attr.s()
class C(object):
first_name = attr.ib() + 'json:"firstName"'
last_name = attr.ib(tag='json:"lastName" sql:"lastname"')

I'll fork and try how I like something like this for my own use. I think
the first class attribute annotations solve this for me as well, but I need
a placeholder for now and the + 'tag' may temporarily hold me over


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#63 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAe2AAvSE9KWAN6yhY8FB3T3SMcxnpkqks5qlEU1gaJpZM4Jm0qX
.

@Tinche
Copy link
Member

Tinche commented Aug 31, 2016

Yes, adding a metadata attribute to _CountingAttr and Attribute is likely the way to go. If you want to be careful, use a frozen class, if not, just use a dict.

Pretty syntax is, then, easy:

@attr.s(slots=True, frozen=True)
class TypeMetadata:
    type = attr.ib()

def typed(t, **kwargs):
    kwargs['metadata'] = TypeMetadata(type=t)
    return _CountingAttr(**kwargs)

def typechecked(t, **kwargs):
    c_attr = typed(t, **kwargs)
    c_attr.validator = instance_of(t)  # Handle existing validator?

@attr.s
def MyClass:
    x = typed(int, validator=.., convert=...)
    y = typechecked(int, convert=...)

print(fields(MyClass(1, 2))[0].metadata.type)
int

@hynek Thoughts?

@Tinche
Copy link
Member

Tinche commented Aug 31, 2016

I'm my example there's actually no need to reach deep down for _CountingAttr (and it doesn't have optional parameters anyway), just wrapping attrs.attr is enough.

Also just for clarification, TypeMetadata, typed and typechecked in the example is user-written functionality. Although I think typed and typechecked would actually be useful in attrs itself, but that's a matter for another issue/PR.

@hynek hynek changed the title User-supplied metatdata for attr.ib User-supplied metadata for attr.ib Sep 1, 2016
@hynek
Copy link
Member

hynek commented Sep 1, 2016

so um is everyone on board with attr.ib(metadata=something_hashable)?

@Tinche
Copy link
Member

Tinche commented Sep 1, 2016

I can put together a PR. Do you want to assert it's hashable? We don't check for any of the other fields.

@hynek
Copy link
Member

hynek commented Sep 1, 2016

I can put together a PR. Do you want to assert it's hashable? We don't check for any of the other fields.

I couldn’t care less what people put in there; it’s a 100% user’s field. I just thought there might be breakage if it isn’t.

OTOH this way is completely non-composable, right? A dict would be freer—multiple extensions could make us of it. Can anyone come up with use cases that might matter?

@Tinche
Copy link
Member

Tinche commented Sep 1, 2016

Yes, at first glance it's non-composable. This was what was tripping me up mentally at the beginning and why I mentioned the lamentable lack of frozendict in an earlier comment.

So let's consider a likely example of composing metadata. You're using two libraries which make use of some custom metadata. Let's pretend one of the libraries wants to know the Python type of your field, and the other wants to know what name to use to for the field when dumping to JSON.

Option one, with a dict:

# Library one, docs say it needs the 'type' field in the metadata
# Library two, docs say it needs 'to_json'
@attr.s
class MyClass:
    x = attr.ib(metadata={'to_json': 'X', 'type': int})

Option two, with classes:

# Library one:
@attr.s(frozen=True)
class TypeMetadata:
    type = attr.ib()

# Library two:
@attr.s(frozen=True)
class JsonMetadata:
    to_json_name = attr.ib()

# My app:
@attr.s
class MyMetadata(TypeMetadata, JsonMetadata):
    pass

@attr.s
class MyClass:
    x = attr.ib(metadata=MyMetadata(to_json_name='X', type=int))

Option two is more boilerplatey, and composes as much as classes compose (which if you're using simple attrs classes, is quite enough). Option two also has the benefits of structured data, so if you mistype 'as_json' instead of 'as_json_name', it'll blow up right away instead of not working silently, and if two libraries expect the same attribute name in their metadata it'll blow up early too. The dict approach isn't 100% composable either, the fields would have to be prefixed with their library name or namespaced in some other way to avoid clashes?

If you're mixing in more metadata, 3+, it'd be more complex and I would probably make my own wrapper for attr.ib with some logic inside.

Another solution would be to use a list of metadata? ¯\ (ツ)_/¯

@hynek
Copy link
Member

hynek commented Sep 1, 2016

paging @glyph :)

@pgruenbacher
Copy link

I really liked @Tinche 's idea for composing the metadata option 2. I'd suggest meta instead of metadata to save the 4 keystrokes, though I'd probably be using wrappers anyways to keep the meta declaration as terse as possible

@wearpants
Copy link
Author

I am very much in favor of @Tinche's Option 1 in my own code; I can never imagine writing something like Option 2 but I can imagine other folks on this thread will want to. From where I sit, it's not attr's business to dictate how the meta shoulld be used - that's the whole reason it's metadata. :-) Just hang on to whatever the user passed in but otherwise ignore it internally.

Along those lines, metadata shouldn't be in the hash & recommended-read-only but not enforced.

@hynek
Copy link
Member

hynek commented Sep 1, 2016

FWIW option two made me throw up a little. 🙈

Attrs will never contain anything that requires you to do multiple inheritance. 😱

@Tinche
Copy link
Member

Tinche commented Sep 1, 2016

Yes, mixins are... an acquired taste. Can we come up with something better than both options?

Or, you know, just let the field be free and let the ecosystem and your own conscience dictate what you put in it.

@wearpants
Copy link
Author

Scrapy has a dict meta attribute on it's Request/Response objects (that
it propagates for you in reasonable ways) used for similar purposes. (I
mention this only b/c I've been using it lately, not b/c I think it's the
end-all-be-all).

The nice thing about a dict is that it's basically a namespace, as opposed
to a single-valued-attribute... but I guess it's just a question of degree
of loosey-gooseyness at that point.

On Thu, Sep 1, 2016 at 4:18 PM, Tin Tvrtković notifications@github.com
wrote:

Yes, mixins are... an acquired taste. Can we come up with something better
than both options?

Or, you know, just let the field be free and let the ecosystem and your
own conscience dictate what you put in it.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#63 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAe2AHcF8QbEpmi6B_knqgFlGqZ5NmtHks5qlzMRgaJpZM4Jm0qX
.

Pete Fein | wearpants.org | @wearpants

@Tinche
Copy link
Member

Tinche commented Sep 1, 2016

Brainstorming a little more.

  • Using classes for metadata is always going to be more verbose than using a dict. It depends on how much we're bothered by this and how much our metadata libraries help us.
  • This metadata field doesn't have to be something that's necessarily world readable directly. There's nothing stopping us from having a helper function; attr.get_metadata(cl, metadata_type). Again, our hypothetical world where we want two metadata types:
from typebla.attr import attr_type  # To be a little diverse.
from jsonbla import JsonMetadata

@attr.s
class MyClass():
    x = attr.ib(metadata=[attr_type(int), JsonMetadata(to_json_name='X')]

attr.get_metadata(MyClass, TypeMetadata)
> TypeMetadata(type=int)
attr.get_metadata(MyClass, JsonMetadata)
> JsonMetadata(to_json_name='X')
attr.get_metadata(MyClass, SomeOtherMetadata)
> None

My gut feeling is really against shortening 'Metadata' to 'Meta' since it makes me think of metaclasses. (For example, from Python 3.5 typing module, AnyMeta, UnionMeta, GenericMeta.)

attr.ib() could be clever and handle both an iterable metadata and a single metadata, so it wouldn't always have to be a list.

@glyph
Copy link
Contributor

glyph commented Sep 6, 2016

I don't like the idea of using classes like this. It seems rather heavyweight when all we need is namespacing, and attributes should almost always be data rather than behavior. In particular I do not like using __class__ or the like as a "key" to lookup metadata, because I'm going to want to use interfaces there most of the time.

Since I hate the vague use of the term "heavyweight", let me be specific :). It's a lot of typing to declare a class with fields and a docstring and such when json_name=... would do. It also causes tight binding to specific implementations, rather than allowing conventions to evolve that are shared between libraries.

If all the metadata needs to be supplied at attribute-construction time, then the possibility of namespace collisions is fairly minimal, and there's always the possibility of, e.g.:

from typebla.attr import attr_type  # To be a little diverse.
from jsonbla import to_json_name

@attr.s
class MyClass():
    x = attr.ib(metadata={attr_type: int, to_json_name: 'X'})

attr.get_metadata(MyClass, TypeMetadata)
>>> TypeMetadata(type=int)
attr.get_metadata(MyClass, JsonMetadata)
>>> JsonMetadata(to_json_name='X')
attr.get_metadata(MyClass, SomeOtherMetadata)
>>> None

if you want to get fancy and cooperate with other libraries.

All in all though, I don't know. I can't think of a solution that fits together really neatly for me and doesn't require a huge amount of ceremony in the default "easy" case. "any hashable object" at least allows for polite libraries to namespace, quick-and-dirty prototypes work, and makes attrs not really care one way or another.

@hynek
Copy link
Member

hynek commented Sep 7, 2016

IOW we’re agreeing it should be a dict? Maybe we define a class MetaData() that is basically a dict but allows for nicer syntax:

@attr.s
class C:
    x = attr.ib(metadata=MetaData(attr_type=int, to_json_name="X")

and gives us some wiggle space, if want to implement magic?

@Tinche
Copy link
Member

Tinche commented Sep 7, 2016

Okay, going with the dict idea as the most user-friendly.

I think the magic can be done when converting _CountingAttr to Attribute anyway (or directly in the ib() function), so we can save ourselves some typing. Also dicts are more capable than kwargs in __init__. (You can't have a non-str key in **kwargs, which might be a way to implement a hook later on.)

@glyph
Copy link
Contributor

glyph commented Sep 7, 2016

IOW we’re agreeing it should be a dict? Maybe we define a class MetaData() that is basically a dict but allows for nicer syntax:

You can also just do dict(attr_type=int, to_json_name="X") for the nicer syntax. :)

@glyph
Copy link
Contributor

glyph commented Sep 7, 2016

I don't like the idea of having a type-with-kwargs as the type of the parameter though, because it makes it slightly harder to do namespacing; I want to be able to have fully qualified Python names ("i.e. strings with dots in") as keys in this dictionary.

@hynek
Copy link
Member

hynek commented Sep 8, 2016

OK whatever, dict it is.

@Tinche
Copy link
Member

Tinche commented Sep 17, 2016

How about I put together a PR for this and we can continue the discussion there?

@wearpants
Copy link
Author

Less talk more rock

On Sep 17, 2016 10:43 AM, "Tin Tvrtković" notifications@github.com wrote:

How about I put together a PR for this and we can continue the discussion
there?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#63 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAe2AF72dVNI1GKIRHHDrtUrJJuDo5FHks5qq_ymgaJpZM4Jm0qX
.

@hynek
Copy link
Member

hynek commented Nov 20, 2016

fixed by #96

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

5 participants