coderholic / django-cities

Countries and cities of the world for Django projects
MIT License
920 stars 374 forks source link

CustomCountryModel not working #165

Open bouncyweirdo opened 7 years ago

bouncyweirdo commented 7 years ago

Checklist

Steps to reproduce

Fresh install, add CustomCountryModel as it is in the example from django-cities, run makemigrations.

INSTALLED_APPS = (
    ...
    # I tried both order
    'myapp',
    'cities',
)

Environment: Python 3.5.2 Django==1.10.5 django-cities==0.5.0.3 ( installed from github, branch master )

Expected behavior

I think it should do the migration files :)

Actual behavior

host:~#./manage.py makemigrations

SystemCheckError: System check identified some issues:

ERRORS:
<myapp>.CustomCountryModel.alt_names: (fields.E300) Field defines a relation with model 'AlternativeName', which is either not installed, or is abstract.
<myapp>.CustomCountryModel.alt_names: (fields.E307) The field <myapp>.CustomCountryModel.alt_names was declared with a lazy reference to '<myapp>.alternativename', but app '<myapp>' doesn't provide model 'alternativename'.
<myapp>.CustomCountryModel_alt_names.alternativename: (fields.E307) The field <myapp>.CustomCountryModel_alt_names.alternativename was declared with a lazy reference to '<myapp>.alternativename', but app '<myapp>' doesn't provide model 'alternativename'.

For a quick fix I moved the AlternativeNames model above Place and removed the lazy reference of AlternativeName

Let me know if you need more details.

blag commented 7 years ago

Instead of moving the AlternativeName model above Place and making the reference a concrete one, what happens if you simply change the lazy reference to cities.AlternativeName?

 @python_2_unicode_compatible
 class Place(models.Model):
     name = models.CharField(max_length=200, db_index=True, verbose_name="ascii name")
-    alt_names = models.ManyToManyField('AlternativeName')
+    alt_names = models.ManyToManyField('cities.AlternativeName')

    objects = models.GeoManager()

Does that fix it?

bouncyweirdo commented 7 years ago

I tried this way already and I get other errors

ValueError: The field cities.City.country was declared with a lazy reference to 'core.customcountrymodel', but app 'core' isn't installed.
The field cities.Country.neighbours was declared with a lazy reference to 'core.customcountrymodel', but app 'core' isn't installed.
The field cities.PostalCode.country was declared with a lazy reference to 'core.customcountrymodel', but app 'core' isn't installed.
The field cities.Region.country was declared with a lazy reference to 'core.customcountrymodel', but app 'core' isn't installed.

But the app core is installed

INSTALLED_APPS = (
    'django.contrib.admin',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.messages',
    'django.contrib.staticfiles',
    'core',
    'cities',
)
blag commented 7 years ago

Dang, I was hoping it would be an easy fix.

If you make a PR with your changes I'll review it and merge it in.

As for making sure this doesn't break in the future, since these issues were caught by Django's checks framework, I'm thinking I can just make an app with custom/swapped models and run Django's check management command.

bouncyweirdo commented 7 years ago

I did more tests, it seems my fix didn't work at all, when it worked I deleted all django-cities migrations and I created them with makemigrations, this way your solution also work, I'll do more tests when I have time, this is a feedback for you in case you have more knowledge about migration files from django-cities.

Both fixes have the same error, so your fix is the right one.

ValueError: The field cities.City.country was declared with a lazy reference to 'core.customcountrymodel', but app 'core' isn't installed.
The field cities.Country.neighbours was declared with a lazy reference to 'core.customcountrymodel', but app 'core' isn't installed.
The field cities.PostalCode.country was declared with a lazy reference to 'core.customcountrymodel', but app 'core' isn't installed.
The field cities.Region.country was declared with a lazy reference to 'core.customcountrymodel', but app 'core' isn't installed.
merwok commented 7 years ago

Hello! I was hoping to use a custom city model to add a multi-polygon field (see #125) and ran into this problem.

what happens if you simply change the lazy reference to cities.AlternativeName?

I get the errors complaining that myapp (containing custom City model) is not installed (it is). The same thing happens if I move the AlternativeName class and change the string reference to an object reference.

@valentinolaru When you say “your fix is the right one”, I don’t understand what you refer to.

stevelacey commented 7 years ago

Yeah these swappable models doesn't seem to work for me either, I get similar error to above when swapping out Country and Continent.

stevelacey commented 7 years ago

After changing the lazy AlternativeName reference to cities.AlternativeName I get the following errors:

ValueError: The field cities.City.country was declared with a lazy reference to 'coworkations.country', but app 'coworkations' doesn't provide model 'country'.
The field cities.Country.continent was declared with a lazy reference to 'coworkations.continent', but app 'coworkations' doesn't provide model 'continent'.
The field cities.Country.neighbours was declared with a lazy reference to 'coworkations.country', but app 'coworkations' doesn't provide model 'country'.
The field cities.PostalCode.country was declared with a lazy reference to 'coworkations.country', but app 'coworkations' doesn't provide model 'country'.
The field cities.Region.country was declared with a lazy reference to 'coworkations.country', but app 'coworkations' doesn't provide model 'country'.
The field coworkations.Trip.country was declared with a lazy reference to 'coworkations.country', but app 'coworkations' doesn't provide model 'country'.
The field coworkations.User.country was declared with a lazy reference to 'coworkations.country', but app 'coworkations' doesn't provide model 'country'.

As per the instructions I am defining the swappables like so:

models.py

class Continent(BaseContinent, models.Model):
    class Meta(BaseContinent.Meta):
        pass

class Country(BaseCountry, models.Model):
    class Meta(BaseCountry.Meta):
        pass

settings.py

CITIES_CONTINENT_MODEL = 'coworkations.Continent'
CITIES_COUNTRY_MODEL = 'coworkations.Country'
blag commented 7 years ago

Try adding swappable = swapper.swappable_setting('cities', 'Continent') to your custom models. See cities/models.py line 101 for an example.

AlexProfi commented 6 years ago

all this variants not work only variant with swapper in meta work without add to settings file CITIES_COUNTRY_MODEL = 'geo.CustomCountryModel' but there are no custom fields in model may be migrations made by django wrong

-- coding: utf-8 --

Generated by Django 1.10 on 2017-07-31 16:44

from future import unicode_literals

from django.conf import settings from django.db import migrations, models import django.db.models.deletion

class Migration(migrations.Migration):

initial = True

dependencies = [
    migrations.swappable_dependency(settings.CITIES_CONTINENT_MODEL),
    ('cities', '0012_auto_20170731_1926'),
    migrations.swappable_dependency(settings.CITIES_COUNTRY_MODEL),
]

operations = [
    migrations.CreateModel(
        name='CustomCountryModel',
        fields=[
            ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
            ('slug', models.CharField(blank=True, max_length=255, null=True)),
            ('name', models.CharField(db_index=True, max_length=200, verbose_name='ascii name')),
            ('code', models.CharField(db_index=True, max_length=2, unique=True)),
            ('code3', models.CharField(db_index=True, max_length=3, unique=True)),
            ('population', models.IntegerField()),
            ('area', models.IntegerField(null=True)),
            ('currency', models.CharField(max_length=3, null=True)),
            ('currency_name', models.CharField(blank=True, max_length=50, null=True)),
            ('currency_symbol', models.CharField(blank=True, max_length=31, null=True)),
            ('language_codes', models.CharField(max_length=250, null=True)),
            ('phone', models.CharField(max_length=20)),
            ('tld', models.CharField(max_length=5, verbose_name='TLD')),
            ('postal_code_format', models.CharField(max_length=127)),
            ('postal_code_regex', models.CharField(max_length=255)),
            ('capital', models.CharField(max_length=100)),
            ('more_data', models.TextField()),
            ('alt_names', models.ManyToManyField(to='cities.AlternativeName')),
            ('continent', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='countries', to=settings.CITIES_CONTINENT_MODEL)),
            ('neighbours', models.ManyToManyField(related_name='_customcountrymodel_neighbours_+', to=settings.CITIES_COUNTRY_MODEL)),
        ],
        options={
            'verbose_name_plural': 'countries',
            'swappable': 'CITIES_COUNTRY_MODEL',
            'abstract': False,
            'ordering': ['name'],
        },
    ),
]

more_data field exists in migration but not in db

blag commented 6 years ago

This may be because django-cities does not yet define an AppConfig. I'll try to fix that later this week and report back.

moorchegue commented 6 years ago

Same when swapping City model.

stevelacey commented 6 years ago

This still doesn't work, if anyone was wondering 🙃

blag commented 6 years ago

I don't have a lot of free time anymore (new job), so I won't be able to get to this for awhile. Can somebody whip up a PR adding an AppConfig to see if that fixes the problem?

rodrivel commented 5 years ago

Hi! Same problem here. Has anyone fixed it?

leibowitz commented 5 years ago

see https://github.com/coderholic/django-cities/pull/203

mhamzawey commented 5 years ago

Any news about this issue?

blag commented 5 years ago

I do not have as much time for this project as I once did. I have contacted the owner about transferring maintainership to somebody else, but as of this writing, he has not responded.

@mhamzawey @rodrivel I would happily accept a pull request that fixed this. At this point, that's going to be the fastest way to get this fixed.

ro5k0 commented 5 years ago

I'm also getting the issue and by the sounds of it, this project is now becoming laid to rest. From what I could test it appears the migrations are somehow causing the issues above (as well as the fix in #208)

blag commented 5 years ago

@ro5k0 This project is not being "laid to rest". I believe @adamhaney is the new maintainer.

I simply don't consistently have time to maintain this project anymore, so it was time for somebody else to maintain it.

ro5k0 commented 5 years ago

Sorry @blag I was just going on the massive text in the README that states

CURRENTLY UNMAINTAINED - contact coderholic to take over maintainership of this project

and the fact this issue has been open since 2017. Those two things often mean the software gets laid to rest from experience. I'm not trying to take anything away from the stoic, invested work you and your fellow collaborators have put in for free to help this project grow. I do in-fact thank you. Great news if @adamhaney is now coming on board as the new maintainer. As mentioned above I believe the ongoing issue is related to the migrations as others have mentioned.

zvolsky commented 3 years ago

@leibowitz #203 : doesn't help me to start, commented at PR #203 So I have no success with custom models and will use basic models + 1:1 tables with additional fields.

Gokujo commented 3 years ago

Instead of moving the AlternativeName model above Place and making the reference a concrete one, what happens if you simply change the lazy reference to cities.AlternativeName?

 @python_2_unicode_compatible
 class Place(models.Model):
     name = models.CharField(max_length=200, db_index=True, verbose_name="ascii name")
-    alt_names = models.ManyToManyField('AlternativeName')
+    alt_names = models.ManyToManyField('cities.AlternativeName')

    objects = models.GeoManager()

Does that fix it?

It worked fine for me

harshalkumar-ishi commented 5 months ago

Instead of moving the AlternativeName model above Place and making the reference a concrete one, what happens if you simply change the lazy reference to cities.AlternativeName?

 @python_2_unicode_compatible
 class Place(models.Model):
     name = models.CharField(max_length=200, db_index=True, verbose_name="ascii name")
-    alt_names = models.ManyToManyField('AlternativeName')
+    alt_names = models.ManyToManyField('cities.AlternativeName')

    objects = models.GeoManager()

Does that fix it?

It worked fine for me

After following this fix i was getting another issue as follows:

django.db.migrations.exceptions.CircularDependencyError: cities.0005_add_foreignkeys_to_postalcode, cities.0006_typify_alt_names_and_add_is_historic, cities.0007_add_currency_and_postal_code_fields_to_country_model, cities.0008_add_code_to_district, cities.0009_add_slug_fields_to_models, cities.0010_adjust_unique_attributes, cities.0011_auto_20180108_0706, cities.0012_alter_alternativename_id_alter_city_id_and_more, address.0001_initial, cities.0002_continent_models_and_foreign_keys, cities.0003_add_verbose_name_and_related_names, cities.0004_rename_languages_to_language_codes