django-multiselectfield icon indicating copy to clipboard operation
django-multiselectfield copied to clipboard

__str__ is broken for MSFList

Open devxpy opened this issue 7 years ago • 9 comments

If the choices are defined like this,

    CHOICE1 = '1'
    CHOICE2 = '2'
    CHOICE3 = '3'
    CHOICE4 = '4'
    ANSWER_CHOICES = ((CHOICE1, 'A'),
                      (CHOICE2, 'B'),
                      (CHOICE3, 'C'),
                      (CHOICE4, 'D'))

then the str method returns None, None, None ..

it can be easily fixed by replacing [msgl.choices.get(int(i)) if i.isdigit() else msgl.choices.get(i) for i in msgl]

with

[msgl.choices.get(i) for i in msgl.choices if i in self.msgl]

devxpy avatar Jan 14 '18 20:01 devxpy

I can't fix this because it would break what other people expect __str__ to serialize.

Why can't you use integers instead of strings? Like so:

    CHOICE1 = 1
    CHOICE2 = 2
    CHOICE3 = 3
    CHOICE4 = 4
    ANSWER_CHOICES = ((CHOICE1, 'A'),
                      (CHOICE2, 'B'),
                      (CHOICE3, 'C'),
                      (CHOICE4, 'D'))

Keep in mind that this project is a hack on top of Django's MultipleChoiceField, and will have problems and limitations due to that.

If you cannot use integers, I would recommend using Django's normal many to many relation for this. Sorry I don't have better advice.

blag avatar Jan 15 '18 06:01 blag

I tried doing that, but I have existing data and it wont convert the types when i migrate, for the existing data. Anyhow, i resorted to using a custom method on my model that does the job :)

devxpy avatar Jan 15 '18 15:01 devxpy

I tried doing that, but I have existing data and it wont convert the types when i migrate, for the existing data

Yep, this a major downside to using this project - it becomes difficult if not impossible to upgrade the database around it. 🙁

I'm glad you found a workaround though. Cheers!

blag avatar Jan 29 '18 04:01 blag

I remember now why i did '1' instead of just 1. Because it won't let me save manually from a script with integers as the input.

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/vagrant/quiz/helper_scripts.py", line 188, in populate_db
    owner=user,
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/query.py", line 417, in create
    obj.save(force_insert=True, using=self.db)
  File "/vagrant/quiz/Exams/models.py", line 70, in save
    super().save(*args, **kwargs)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/base.py", line 729, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/base.py", line 759, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/base.py", line 842, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/base.py", line 880, in _do_insert
    using=using, raw=raw)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/query.py", line 1125, in _insert
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1280, in execute_sql
    for sql, params in self.as_sql():
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1233, in as_sql
    for obj in self.query.objs
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1233, in <listcomp>
    for obj in self.query.objs
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1232, in <listcomp>
    [self.prepare_value(field, self.pre_save_val(field, obj)) for field in fields]
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1172, in prepare_value
    value = field.get_db_prep_save(value, connection=self.connection)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/django/db/models/fields/__init__.py", line 767, in get_db_prep_save
    return self.get_db_prep_value(value, connection=connection, prepared=False)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/multiselectfield/db/fields.py", line 140, in get_db_prep_value
    value = self.get_prep_value(value)
  File "/home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/multiselectfield/db/fields.py", line 136, in get_prep_value
    return '' if value is None else ",".join(value)
TypeError: sequence item 0: expected str instance, int found

devxpy avatar Feb 16 '18 10:02 devxpy

Reopened because this is something I might be able to fix.

I can simply convert all keys to strings, would that work for you?

Try modifying the file in the Vagrant container - change line 136 of /home/vagrant/.pyenv/versions/venv/lib/python3.6/site-packages/multiselectfield/db/fields.py to this:

        return '' if value is None else ",".join(value)

and see if that works. If so I'll just have it convert all keys to strings.

blag avatar Feb 17 '18 00:02 blag

I am working on moving a Django project on py2 to py3 and I got an error related to this, I had some sql migrations produced from changing python versions and removing from __future__ import unicode_literals

in lib/python3.6/site-packages/multiselectfield/db/fields.py", line 136
    return '' if value is None else ",".join(value)
TypeError: sequence item 0: expected str instance, int found

it looks like the code you suggested is already in there.

The sql migration code produced:

migrations.AlterField(
            model_name='event',
            name='employee_type',
            field=multiselectfield.db.fields.MultiSelectField(choices=[('i', 'Interpreters'), ('c', 'RTC-NT'), ('n', 'Other')], default='i', max_length=4),
        ),

audiolion avatar Feb 20 '18 14:02 audiolion

@blag It would have been better if this used some standard serialization format like JSON, or at least python's __repr__(), so that it differentiates between an int and str objects.

But that would break everything.

Why not test if the CHOICES have an int or str, and do type conversion based on that?

Also, why use .get() as opposed to __getitiem__(), which returns an ambiguous None instead of an exception?

devxpy avatar Sep 10 '18 10:09 devxpy

+1

mohammedhammoud avatar Nov 21 '18 23:11 mohammedhammoud

I'm not sure if this was mentioned here, but in case it helps anyone, this breaks printing out Django model fields, like:

object = Object.objects.get(id=id)
print(object.field)

the internal representation, however, is available: repr(object.field)

So if the issue is not resolved, the string values of MSFList can be accessed with repr()

erikjyrmann avatar Oct 01 '19 13:10 erikjyrmann