Renaming apps.has_app

[Aymeric Augustin]

Hello,

During the app_loading refactor, I introduced a method to test if a given application is enabled, and I named it `has_app`.

Its main uses are detecting misconfigurations:

if not apps.has_app('django.contrib.admin'):
raise ImproperlyConfigured("Put 'django.contrib.admin' in your "
"INSTALLED_APPS setting in order to use the admin application.")

and skipping tests:

@skipUnless(apps.has_app('django.contrib.sites'),
"django.contrib.sites app not installed.")

That method takes an application name like “django.contrib.admin” rather than a label for two reasons: name conflicts are less likely than label conflicts (but they’re still possible) and unlike labels names cannot be changed.

`apps.has_app(...)` is technically correct but I think we can find a better name. My current favorite is `apps.installed(…)`. It’s quite short and it’s reminiscent of INSTALLED_APPS. That makes (some) sense since the method tests if its argument is in INSTALLED_APPS, accounting for AppConfigs.

Since I’m awful at picking names, I’d like to hear your suggestions and arguments before making a decision.

Thank you,

--
Aymeric.

[Jannis Leidel]

+1 on apps.installed(..)

Jannis

[Josh Smeaton]

+1 on .installed() as it reads quite well. The only thing I have against it is that it may sound like it can take an iterable of app names, where has_app() does not have that problem. app_installed() is more accurate but is longer and less nice to read. My 2 cents.

Cheers,

Josh

[Aymeric Augustin]

On 5 janv. 2014, at 22:27, Josh Smeaton <josh.s...@gmail.com> wrote:

> The only thing I have against it is that it may sound like it can take an iterable of app names, where has_app() does not have that problem.

What about is_installed?

--
Aymeric.

[Raffaele Salmaso]

On Sun, Jan 5, 2014 at 10:11 PM, Aymeric Augustin
<aymeric....@polytechnique.org> wrote:
> `apps.has_app(...)` is technically correct but I think we can find a better name. My current favorite is `apps.installed(…)`. It’s quite short and it’s reminiscent of INSTALLED_APPS. That makes (some) sense since the method tests if its argument is in INSTALLED_APPS, accounting for AppConfigs.
>
> Since I’m awful at picking names, I’d like to hear your suggestions and arguments before making a decision.

apps.is_installed('django.contrib.sites') ?

Should it takes a list of apps, just to replace
apps.is_installed('djagno.contrib.sites') and
apps.is_installed('django.contrib.admin') and ...
with
apps.is_installed('django.contrib.sites', 'django.contrib.admin',...)
?

--
| Raffaele Salmaso
| http://salmaso.org
| https://bitbucket.org/rsalmaso
| http://gnammo.com

[Aymeric Augustin]

On 5 janv. 2014, at 22:38, Raffaele Salmaso <raff...@salmaso.org> wrote:

> Should it takes a list of apps, just to replace
> apps.is_installed('djagno.contrib.sites') and
> apps.is_installed('django.contrib.admin') and ...
> with
> apps.is_installed('django.contrib.sites', 'django.contrib.admin',...)
> ?

That’s an interesting suggestion. But I don’t think it’s compatible with the singular “is_installed”. It could work with just “installed”, though.

--
Aymeric.

[Shai Berger]

I'd go for __contains__:

if "django.contrib.auth" in apps:

Shai.

[Kevin Christopher Henry]

+1 for "is_installed"

I don't find the grammar objectionable here, just think of it as "is_(each one of these apps)_installed"

[Aymeric Augustin]

On 5 janv. 2014, at 22:54, Shai Berger <sh...@platonix.com> wrote:

> I'd go for __contains__:
>
> if "django.contrib.auth" in apps:

I considered this one but I didn’t select it because it will restrict our freedom in the future.

If I were to add magic methods on the app registry I’d probably make it a dict of app_label => app_config. This is the most common use case.

Then it would be inconsistent to support `if "django.contrib.auth" in apps` and `apps['admin']` at the same time.

--
Aymeric.

[Shai Berger]

That's a little odd. You noted that names are better keys, which is why they
are used in has_app(). Why would labels be preferable for dictionary access?

That being said, keeping the freedom for later does make sense.

Shai.

[Curtis Maloney]

Is the purpose to test if a particular name is installed, or a particular module?  The distinction could be important now that the two are not directly related.

 

[Jorge Cardoso Leitão]

Hi.

First of all, a hug to Aymeric for this, very nice work!

My first thought was also on "is_installed", which seems natural.

But, on my second thought, the "has_app" is not that bad because it is a method of the "app loading"; i.e. the apps "having" something makes sense than "being" something. The is_installed would make sense if we were picking a specific app and asking if it was installed: "app.is_installed()".

That said, what about "apps.has_installed(app_name)"?

("has" because methods are normally applied to singular instances, but since "apps" is a plural name, "have_installed" would actually make more sense).

Cheers,

Jorge

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/A2964B6C-66B2-4023-A3CE-34BBC812D025%40polytechnique.org.

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

[Russell Keith-Magee]

is_installed() works for me.

Russ %-) 

 

[Luc Saffre]

On 06/01/14 00:26, Aymeric Augustin wrote:
> On 5 janv. 2014, at 22:54, Shai Berger <sh...@platonix.com> wrote:
>
>> I'd go for __contains__:
>>
>> if "django.contrib.auth" in apps:
>

> I considered this one but I didn�t select it because it will restrict our freedom in the future.
>
> If I were to add magic methods on the app registry I�d probably make it a dict of app_label => app_config. This is the most common use case.

>
> Then it would be inconsistent to support `if "django.contrib.auth" in apps` and `apps['admin']` at the same time.
>

+1 for is_installed.

And I like your plan to make apps behave as a dict of app_label =>
AppConfig instances. When you want to test for a full app name, use

if apps.is_installed('django.contrib.admin'): ...

If you don't care about the full path and want to test for the
app_label, then use

if "admin" in apps: ...

Aymeric, you are really talented! Thanks for your work!
Luc

[German Larrain]

+1 for is_installed

Aymeric, thanks for your work

[Andre Terra]

On Sun, Jan 5, 2014 at 7:38 PM, Raffaele Salmaso <raff...@salmaso.org> wrote:

On Sun, Jan 5, 2014 at 10:11 PM, Aymeric Augustin
<aymeric....@polytechnique.org> wrote:
> `apps.has_app(...)` is technically correct but I think we can find a better name. My current favorite is `apps.installed(…)`. It’s quite short and it’s reminiscent of INSTALLED_APPS. That makes (some) sense since the method tests if its argument is in INSTALLED_APPS, accounting for AppConfigs.
>
> Since I’m awful at picking names, I’d like to hear your suggestions and arguments before making a decision.

apps.is_installed('django.contrib.sites') ?

Should it takes a list of apps, just to replace
apps.is_installed('djagno.contrib.sites') and
apps.is_installed('django.contrib.admin') and ...
with
apps.is_installed('django.contrib.sites', 'django.contrib.admin',...)
?

The only issue I see with this solution is that it's not clear at first whether this is an AND or OR match, plural agreement notwithstanding.

Cheers,

AT