Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#20477 closed New feature (fixed)

Allow list of modules for FORMAT_MODULE_PATH

Reported by: mbrochh Owned by: mbrochh
Component: Internationalization Version: master
Severity: Normal Keywords: localization, format files
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently the FORMAT_MODULE_PATH setting (see https://docs.djangoproject.com/en/dev/topics/i18n/formatting/#creating-custom-format-files) only allows a string as a value. This is not optimal because it prevents us from shipping custom format strings with reusable apps.

Imagine the following situation:

  • You have created a reusable app that provides a new custom format string for displaying person names
  • You have created a reusable app that heavily relies on the format string for SHORT_DATE to be in a specific format and thus provides a formats folder
  • In your django project you would like to override some of the other standard format strings

So in this situation you would have one project and two apps each having a formats folder. But we can only set FORMAT_MODULE_PATH to one path.

The code that currently loads those format strings is actually able to deal with a list (see https://github.com/django/django/blob/master/django/utils/formats.py#L67), because first it loads Django's standard formats (see https://github.com/django/django/blob/master/django/utils/formats.py#L46) and then it loads the module defined in FORMAT_MODULE_PATH. It then reverses the list, so that our setting overrides the values of Django's default.

In order to solve this problem, we have copied all this format string loading code and basically just changed line 47, turning the if-clause into a for loop (see https://github.com/bitmazk/django-libs/blob/master/django_libs/format_utils.py#L51).

Now we can set the setting like so:

CUSTOM_FORMAT_MODULE_PATH = [

'my_django_project.formats',
'my_person_name_app.formats',
'my_short_date_app.formats',

]

After working with Django professionally since more than 2 years, I would love to make this my first contribution. If I provided a patch (with tests and documentation), would there be any chance that it will be merged?

Change History (11)

comment:1 Changed 3 years ago by Claude Paroz

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

Yes, it makes sense for me.

comment:2 Changed 3 years ago by mbrochh

Owner: changed from nobody to mbrochh
Status: newassigned

comment:3 Changed 3 years ago by Martin Brochhaus <mbrochh@…>

Created pull request: https://github.com/django/django/pull/1236

I actually only replaced one line of Django code with a few more lines. The change is quite trivial.
The original code was not yet covered by the test suite, so I added new tests and increased the test coverage for utils/formats.py.
I also updated to docs.

Hope this is all good :)

comment:4 Changed 3 years ago by Martin Brochhaus <mbrochh@…>

Has patch: set

comment:5 Changed 3 years ago by Martin Brochhaus <mbrochh@…>

Update: I added some comments to the pull request to make codereview easier.

comment:6 Changed 3 years ago by Tim Graham

Patch needs improvement: set

There are comments on the pull request indicating it needs improvement.

comment:7 Changed 3 years ago by Tim Graham

Easy pickings: unset

comment:8 Changed 3 years ago by mbrochh

@timo I have done the requested improvement: https://github.com/django/django/pull/1236

comment:9 Changed 2 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 950b6de16ac2f8135612f2ed5984c090dd8e4dcf:

Fixed #20477: Allowed settings.FORMAT_MODULE_PATH to be a list of modules.

Previously the FORMAT_MODULE_PATH setting only accepted one string (dotted
module path).

This is useful when using several reusable third party apps that define new
formats. We can now use them all and we can even override some of the formats
by providing a project-wide format module.

comment:10 Changed 2 years ago by Tim Graham <timograham@…>

In 5abc811a40dbeee092b9f50055d229e9c24274fa:

Revert "Fixed #20477: Allowed settings.FORMAT_MODULE_PATH to be a list of modules."

This reverts commit 950b6de16ac2f8135612f2ed5984c090dd8e4dcf.

comment:11 Changed 2 years ago by Tim Graham <timograham@…>

In bb0a9a070b9570c85bcb17346dae6513e4ba6e76:

Fixed #20477: Allowed list of modules for FORMAT_MODULE_PATH

Previously the FORMAT_MODULE_PATH setting only accepted one string (dotted
module path). A feature has been added to also allow a list of strings.

This is useful when using several reusable third party apps that define new
formats. We can now use them all and we can even override some of the formats
by providing a project-wide format module.

Note: See TracTickets for help on using tickets.
Back to Top