Add ability to set custom kwargs on ChoiceItem

[kavdev]

It's not common behavior, but we have a model that validates data based on a chosen data label. Instead of putting the validator into a dictionary and accessing it ad-hoc, we'd rather just set it on the choice and access it.

This implements the ability to add custom attributes to a choice.

[kavdev]

I'll add tests if this is something you're interested in merging.

[coveralls]

Coverage decreased (-1.02%) to 94.382% when pulling 06a0c0321af6aedea1388952b35089d210249a13 on kavdev:patch-2 into 161f42eace9cd6c098926d369dbe07bedb261a45 on bigjason:master.

[sergei-maertens]

I've had similar needs so far, but the problem is that you currently can only access the ChoiceItem, so accessing that has to become public API as well. I don't see any problems with that though.

Thanks for your contributions, I'm not near a computer until Monday, so I can't properly merge and release yet.

On Jun 16, 2017 02:32, "Alexander Kavanaugh" notifications@github.com wrote:

I'll tests if this is something you're interested in merging.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bigjason/django-choices/pull/52#issuecomment-308899371, or mute the thread https://github.com/notifications/unsubscribe-auth/AFQ01ibHRBIi24F13YKBNau6Y8u3IWeHks5sEcozgaJpZM4N74p6 .

[kavdev]

No worries! I'll be committing tests and packaging updates in a bit. Let me know how you want to handle opening up the fields API and I'll be happy to squeeze that in as well.

[coveralls]

Coverage increased (+0.08%) to 96.0% when pulling efe9bdc3974e2ff2bccdfa3fa1ddaeeba918d5f0 on kavdev:patch-2 into fc6714254ca39c9b563283a2b473a3ac51eae013 on bigjason:develop.

[coveralls]

Coverage increased (+0.08%) to 96.0% when pulling 2f5ba4b63cb72d55699e522c21d327f0b2d69b3d on kavdev:patch-2 into fc6714254ca39c9b563283a2b473a3ac51eae013 on bigjason:develop.

[coveralls]

Coverage increased (+0.08%) to 96.0% when pulling 2f5ba4b63cb72d55699e522c21d327f0b2d69b3d on kavdev:patch-2 into fc6714254ca39c9b563283a2b473a3ac51eae013 on bigjason:develop.

[sergei-maertens]

hey @kavdev, I found some time to take a look at this, and I've got a couple things that I'm leaning towards to. Happy to discuss if you disagree on some aspects!

The ChoiceItem modifications:

1. I think we shouldn't just setattr the custom properties, I'd rather keep them a bit 'protected' in a self._extras attribute. I think this doesn't pollute the namespace too much, and it makes it easier to keep track on what exactly was passed in as custom props (and useful for item 2.). So the init would become more like this:

       def __init__(self, value=sentinel, label=None, order=None, **extra):
           self.value = value
           self.label = label
           self._extra = extra
   
           if order is not None:
               self.order = order
           else:
               ChoiceItem.order += 1
               self.order = ChoiceItem.order

2. Since the introduction of arbitratry props may complicate things, I think it'd be wise to implement the __repr__ method to make debugging easier:

       def __repr__(self):
           extras = " ".join([
               "{key}={value!r}".format(key=key, value=value)
               for key, value in self._extra.items()
           ])
           return "<{} value={!r} label={!r} order={!r}{extras}>".format(
               self.__class__.__name__, self.value,
               self.label, self.order,
               extras=" " + extras if extras else ""
           )

The DjangoChoices class:

Essentially this would be 'opening up' the _fields api, but I'd make it a class method:

    @classmethod
    def get_choice(cls, value):
        """
        Return the underlying :class:`ChoiceItem` for a given value.
        """
        attribute_for_value = cls.attributes[value]
        return cls._fields[attribute_for_value]

The details are in the cls.attributes, which may throw an exception if duplicate values are used.

Example usage would then be:

>>> choice_item = MyChoices.get_choice(MyChoices.foo)

Thoughts?

[kavdev]

@sergei-maertens I like it! I'll hopefully get to it this weekend. I'll create a separate PR for the get_choice method

[sergei-maertens]

If you could put it all in the existing PR that would make things a bit more contained and I'd prefer that :-)

On Jun 21, 2017 23:56, "Alexander Kavanaugh" notifications@github.com wrote:

@sergei-maertens https://github.com/sergei-maertens I like it! I'll hopefully get to it this weekend. I'll create a separate PR for the get_choice method

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bigjason/django-choices/pull/52#issuecomment-310216718, or mute the thread https://github.com/notifications/unsubscribe-auth/AFQ01ucGBPsEU-IDuz5KWfTqtWEv-fPTks5sGZGcgaJpZM4N74p6 .

[kavdev]

@sergei-maertens no problem. whatever works best for you.

[kavdev]

@sergei-maertens thinking about it, the "_extras" protection seems more of a hindrance than a benefit.

If I set:

class MyChoices(DjangoChoices):
    choice_item = ChoiceItem(value=0, label="test", help_text="help!")

I'd expect to be able to use

>>> choice_item = MyChoices.get_choice(MyChoices.choice_item)
>>> choice_item.help_text
'help!'

That's more intuitive and readable than

>>> choice_item._extras["help_text"]

[sergei-maertens]

Looks like I forgot a def __getattr__(self): in my sample code!

On Jun 28, 2017 01:01, "Alexander Kavanaugh" notifications@github.com wrote:

@sergei-maertens https://github.com/sergei-maertens thinking about it, the "_extras" protection seems more of a hindrance than a benefit.

If I set:

class MyChoices(DjangoChoices): choice_item = ChoiceItem(value=0, label="test", help_text="help!")

I'd expect to be able to use

choice_item = MyChoices.get_choice(MyChoices.choice_item)>>> choice_item.help_text'help!'

That's more intuitive and readable than

choice_item._extras["help_text"]

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bigjason/django-choices/pull/52#issuecomment-311509889, or mute the thread https://github.com/notifications/unsubscribe-auth/AFQ01iGnSLKaso2XnipRajfTfs-QF0Qxks5sIYnRgaJpZM4N74p6 .

[kavdev]

Looks like I forgot a def __getattr__(self): in my sample code!

👍 I'll add it

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 4f2802cfe1025b2ce7275c04508a1dbeccf180a0 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 4f2802cfe1025b2ce7275c04508a1dbeccf180a0 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling cb1a0a516edec011cf712904df17dfa0ee2a3e6a on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling cb1a0a516edec011cf712904df17dfa0ee2a3e6a on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 0f602ef1e3afd0cad9cd5aedaa86f6934a3ad581 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 0f602ef1e3afd0cad9cd5aedaa86f6934a3ad581 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 0f602ef1e3afd0cad9cd5aedaa86f6934a3ad581 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 0f602ef1e3afd0cad9cd5aedaa86f6934a3ad581 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 0f602ef1e3afd0cad9cd5aedaa86f6934a3ad581 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[kavdev]

@sergei-maertens coveralls doesn't seem to enjoy rebasing :)

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 48efe7c6fca2d410a1828fe22b75af9d01f77715 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling 48efe7c6fca2d410a1828fe22b75af9d01f77715 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling ccef85a9cf0d617c978ca16d0c9adaca8faf6283 on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[coveralls]

Coverage increased (+0.3%) to 97.273% when pulling a1b748edb3d871d2eb082fc64fa96f0433a2be5d on kavdev:patch-2 into f9cdc9b1953c6cdfaa4d9381fec855837ac0c812 on bigjason:develop.

[sergei-maertens]

it does seem pretty chatty, yes :P

[sergei-maertens]

Thanks for your contribution! Only thing missing is a documentation update. Will you do that as well, or do I need to follow up on that? I'm merging this either way - if you decide to tackle docs, you can submit a new PR for that :-)

[kavdev]

@sergei-maertens I can add it to my list, but I'm pretty swamped; it might be a month or so until it reaches the top

[sergei-maertens]

Alright, I'll handle it!

On Jul 11, 2017 20:15, "Alexander Kavanaugh" notifications@github.com wrote:

@sergei-maertens https://github.com/sergei-maertens I can add it to my list, but I'm pretty swamped; it might be a month or so until it reaches the top

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bigjason/django-choices/pull/52#issuecomment-314528365, or mute the thread https://github.com/notifications/unsubscribe-auth/AFQ01qPHTqb0o-1nRePtLjifg8rRLqNVks5sM7vEgaJpZM4N74p6 .

[kavdev]

Thanks!

[sergei-maertens]

@kavdev 1.6.0 has just been released on PyPI