Re: Removing aliased imports ("from") in generated migration files to prevent conflicts

[Alexey Kotlyarov]

Django doesn't depend on flake8, so it can't be used during migration 
generation. Otherwise, I think aliasing the conflicting modules (6) is 
indeed a better way.

The problem came about on #django IRC channel because someone named an app 
"settings", presumably to store some configuration.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/a15fe07e-2367-4a8a-9fc3-3a06cf84615f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Removing aliased imports ("from") in generated migration files to prevent conflicts

[Adam Johnson]

There's a 6th way, which is to alias the probematic modules rather than
django's modules, e.g.

from django.db import models

import models as models_

...
Field(
default=models_.my_default
)
...

There's also a 7th way, which is to use flake8 which will, with its default
config, complain about duplicate imports. Django could possibly even hook
into flake8 and call it on generated migration files and warn the user
they're bad, although many people already run it as part of their toolchain.

But...

While the above names arguably aren't good choices for a module name


Is this a real world problem? The ticket doesn't report any real world
clash, it just says it's possible. This whole thing seems like a
maybe-possibly edge case, for applications with badly named modules that
would confuse day-to-day development anyway.

I'm -1 on option 3, absolute imports do indeed make generated migrations
harder to read and edit.

On 9 January 2017 at 22:06, Anthony King  wrote:

> There may be a 5th way, which would be to make an alias under the clash
> conditions.
>
> from django.db import models as django_models
>
> However this could also lead to mistakes in assume the django models would
> still be called models.
>
>
> On 9 Jan 2017 21:59, "Alexey Kotlyarov"  wrote:
>
> As stated in https://code.djangoproject.com/ticket/26099, it is possible
> for a project with an unfortunately named application to result in a
> migration file being generated which will raise an error on attempting to
> migrate, without any prior warning. The problem is that the following
> imports can all be generated:
>
> from django.db import migrations, models
> from django.conf import settings
> from django.utils.timezone import utc
>
> If an application module is named models, migrations, settings or utc and
> there is, for example, a function in one of those modules used for a
> default of a field, its module will be imported as is and shadow the Django
> import.
>
> While the above names arguably aren't good choices for a module name, they
> are valid, don't conflict with anything else within Django and the behavior
> isn't mentioned in the documentation.
>
> Here are several proposed solutions to the problem:
>
> 1. Mention the module names causing the bug to manifest in the
> documentation, keep the behavior. This isn't likely to help the affected
> users, and the name list must be kept in sync if new imports are added to
> the generated migrations.
>
> 2. Warn about the name clash, refuse to generate the migration and tell
> the user that they must rename a module. This might not be possible in case
> the offending name is from a third party library.
>
> 3. Generate the imports without aliasing them (e.g. import
> django.db.models) to make clashes impossible. This, however, makes the
> migration files harder to read to a human, as all the imported names, like
> Model and Migration, use full module names: django.db.models.Model,
> django.db.migrations.Migration.
>
> 4. Only generate the un-aliased imports if there is a module name clash.
>
> I have naively gone with option 3 and provided a patch (
> https://github.com/django/django/pull/7810), but Tim Graham pointed out
> that the deficiencies I was pondering too.
>
> What is the best way forward here? I have an understanding of what's
> needed to implement 4 and 2 (won't be much easier than 4) and can change
> the patch accordingly.
>
> Alexey
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/ms
> gid/django-developers/daba352d-5e97-4d70-b61f-b42f9eb65ca5%
> 40googlegroups.com
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/django-developers/CALs0z1YzVLuvGixpm44ohzA9v6GcL
> yk_gAVh57szehoKMymkzQ%40mail.gmail.com
> 

Re: Removing aliased imports ("from") in generated migration files to prevent conflicts

[Anthony King]

There may be a 5th way, which would be to make an alias under the clash
conditions.

from django.db import models as django_models

However this could also lead to mistakes in assume the django models would
still be called models.


On 9 Jan 2017 21:59, "Alexey Kotlyarov"  wrote:

As stated in https://code.djangoproject.com/ticket/26099, it is possible
for a project with an unfortunately named application to result in a
migration file being generated which will raise an error on attempting to
migrate, without any prior warning. The problem is that the following
imports can all be generated:

from django.db import migrations, models
from django.conf import settings
from django.utils.timezone import utc

If an application module is named models, migrations, settings or utc and
there is, for example, a function in one of those modules used for a
default of a field, its module will be imported as is and shadow the Django
import.

While the above names arguably aren't good choices for a module name, they
are valid, don't conflict with anything else within Django and the behavior
isn't mentioned in the documentation.

Here are several proposed solutions to the problem:

1. Mention the module names causing the bug to manifest in the
documentation, keep the behavior. This isn't likely to help the affected
users, and the name list must be kept in sync if new imports are added to
the generated migrations.

2. Warn about the name clash, refuse to generate the migration and tell the
user that they must rename a module. This might not be possible in case the
offending name is from a third party library.

3. Generate the imports without aliasing them (e.g. import django.db.models)
to make clashes impossible. This, however, makes the migration files harder
to read to a human, as all the imported names, like Model and Migration,
use full module names: django.db.models.Model,
django.db.migrations.Migration.

4. Only generate the un-aliased imports if there is a module name clash.

I have naively gone with option 3 and provided a patch (
https://github.com/django/django/pull/7810), but Tim Graham pointed out
that the deficiencies I was pondering too.

What is the best way forward here? I have an understanding of what's needed
to implement 4 and 2 (won't be much easier than 4) and can change the patch
accordingly.

Alexey

-- 
You received this message because you are subscribed to the Google Groups
"Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/
msgid/django-developers/daba352d-5e97-4d70-b61f-b42f9eb65ca5%40googlegroups.
com

.
For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CALs0z1YzVLuvGixpm44ohzA9v6GcLyk_gAVh57szehoKMymkzQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Removing aliased imports ("from") in generated migration files to prevent conflicts

[Alexey Kotlyarov]

As stated in https://code.djangoproject.com/ticket/26099, it is possible 
for a project with an unfortunately named application to result in a 
migration file being generated which will raise an error on attempting to 
migrate, without any prior warning. The problem is that the following 
imports can all be generated:

from django.db import migrations, models
from django.conf import settings
from django.utils.timezone import utc

If an application module is named models, migrations, settings or utc and 
there is, for example, a function in one of those modules used for a 
default of a field, its module will be imported as is and shadow the Django 
import.

While the above names arguably aren't good choices for a module name, they 
are valid, don't conflict with anything else within Django and the behavior 
isn't mentioned in the documentation.

Here are several proposed solutions to the problem:

1. Mention the module names causing the bug to manifest in the 
documentation, keep the behavior. This isn't likely to help the affected 
users, and the name list must be kept in sync if new imports are added to 
the generated migrations.

2. Warn about the name clash, refuse to generate the migration and tell the 
user that they must rename a module. This might not be possible in case the 
offending name is from a third party library.

3. Generate the imports without aliasing them (e.g. import django.db.models) 
to make clashes impossible. This, however, makes the migration files harder 
to read to a human, as all the imported names, like Model and Migration, 
use full module names: django.db.models.Model, 
django.db.migrations.Migration.

4. Only generate the un-aliased imports if there is a module name clash.

I have naively gone with option 3 and provided a patch (
https://github.com/django/django/pull/7810), but Tim Graham pointed out 
that the deficiencies I was pondering too.

What is the best way forward here? I have an understanding of what's needed 
to implement 4 and 2 (won't be much easier than 4) and can change the patch 
accordingly.

Alexey

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/daba352d-5e97-4d70-b61f-b42f9eb65ca5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.