Skip to content

Make multigtfs easily extensible#83

Open
misli wants to merge 1 commit into
tulsawebdevs:masterfrom
bileto:extensible
Open

Make multigtfs easily extensible#83
misli wants to merge 1 commit into
tulsawebdevs:masterfrom
bileto:extensible

Conversation

@misli

@misli misli commented Jun 14, 2018

Copy link
Copy Markdown
Contributor

This change makes multigtfs easily extensible by other Django applications.
All you need is to create model based on multigtfs.models.base.Base
and register the model with decorator multigtfs.models.Feed.register_model.

For example, we use following sexi (Stop External Id) extension in our project:

from jsonfield import JSONField
from multigtfs.models.base import models, Base
from multigtfs.models import Feed, Stop

@Feed.register_model
class StopExternalId(Base):
    """An External Id of the stop.

    This implements stop_external_ids.txt in the GTFS feed.
    """
    stop = models.ForeignKey(Stop, on_delete=models.CASCADE)
    external_type = models.CharField(
        max_length=255, blank=True,
        help_text="Type of the external id")
    external_id = models.CharField(
        max_length=255, blank=True,
        help_text="Value of the external id")
    extra_data = JSONField(default={}, blank=True, null=True)

    def __str__(self):
        return "%s-%s-%s" % (self.stop, self.external_type, self.external_id)

    class Meta:
        app_label = 'sexi'

    _column_map = (
        ('stop_id', 'stop__stop_id'),
        ('type', 'external_type'),
        ('id', 'external_id'),
    )
    _filename = 'stop_external_ids.txt'
    _rel_to_feed = 'stop__feed'
    _sort_order = ('stop__stop_id', 'external_type')
    _unique_fields = ('stop_id', 'type')

@coveralls

coveralls commented Jun 14, 2018

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.8%) to 99.23% when pulling 6f5c788 on bileto:extensible into ca19ee6 on tulsawebdevs:master.

@jwhitlock

Copy link
Copy Markdown
Member

This is interesting, I still need to think about the approach.

I'm surprised the tests passed, since the processing order of imports and exports was different before, but maybe they reflect an issue with real feeds rather than ones from the tests.

Are you using migrations in your project? How did this code interact with migrations?

@misli

misli commented Jun 19, 2018

Copy link
Copy Markdown
Contributor Author

I use the same order for import (because it matters). For export the order de-facto doesn't matter, so first I used the same order for both, but than I realized that I need to sort them by the filename to pass the tests. However, I should still write some tests for the registering / unregistering feature. Hopefully I'll get back to it soon.

@misli

misli commented Jun 19, 2018

Copy link
Copy Markdown
Contributor Author

Regarding the migrations, we use them. As You see in the example, the Meta.app_label needs to be specified for the model to be correctly identified by the migrations tools. (Without the app_label Django assumes that the model belongs to multigtfs.) May be I should add some documentation too.

@jwhitlock

Copy link
Copy Markdown
Member

I like the idea of ordering exports based on the _filename attribute - that seems like a win by itself, and makes the expected ordering more explicit.

I'm trying to think if there is a more "Django" way to register and order the models. The register methods would require some one-time code to setup the extended models, such as in an AppConfig class. I'm thinking if something like the MIDDLEWARE declaration would work, where a default list is used but could be overridden in settings. I may be thinking too far ahead...

@misli

misli commented Sep 11, 2018

Copy link
Copy Markdown
Contributor Author

I see registering using decorator a very Python/Django way. It is very similar to registering to Django admin site for example.
In most cases you will register the models in the right order simply because you declare the models in the same order. In some edge cases you may still explicitly call the register method (not using it as decorator) in the right order.
Unlike using some settings attribute, using this decorator registration makes use of third party extension as simple as adding it to INSTALLED_APPS 😉

@misli

misli commented Mar 24, 2020

Copy link
Copy Markdown
Contributor Author

@daliborpavlovic, do you really mean to update this pull request with the latest changes?
Maybe consider creating another branch for them, based on this one.
@jwhitlock, are You still considering this PR?

@daliborpavlovic

daliborpavlovic commented Mar 25, 2020

Copy link
Copy Markdown
Contributor

No, sorry, I did not want to update this PR. I was just playing around with our fork and did not know, that this would propagate here. I made reset and force push of the original version, if that is ok.

@jspetrak

jspetrak commented Apr 2, 2021

Copy link
Copy Markdown
Contributor

We would like to merge this feature. May also provide general solution for #73 without extending multigtfs by non-standard models.

Before merge, test case must be provided. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants