Pull requests

#1 Merged
Repository
egasimus egasimus
Branch
master
Repository
brankovukelic brankovukelic
Branch
master

Compatibility improvements

Author
  1. egasimus
Reviewers
Description

Zdravo Branko,

Thanks for your quick response. I've made a few tweaks to the code in the meantime, so I can't really provide you with a stack trace anymore - everything just works now! Here's what I did:

  • On Python 2.7.3, I was getting a DeprecationWarning at I18nManager.__new__. I fixed that, possibly improving compatibility with Python 3, which apparently doesn't allow one to pass arguments into __new__, anyway.
  • I've taken the liberty to apply my own coding style to some function calls, lists, etc. With the limited vertical space that the current crop of widescreen monitors give us, I find any piece of code much more comprehensible when space is used more effectively.
  • I've made I18nModelMixin.model to point to the translation model and not the source model to stick with the Django paradigm for inlines. This is what I believe to have sorted out the Reversion bug, and it's likely to prevent many more compatibility in the issues.
  • It seems that Django 1.6 has introduced a get_extra method of its own, using it in place of self.extra when setting defaults in InlineModelAdmin.get_formset. It takes a request parameter before obj, though, and the clashing method signatures were giving me a nasty error. I've updated I18nInlineMixin.get_extra to match InlineModelAdmin.get_extra.

I need to add that I really like your approach to the problem of translating model fields. It was unimaginable that, if I wanted my translations stored in a separate table, I would have to wrap my head around the horrible syntax introduced by something like Hvad or Parler! A quick look at the documentation and you can kind of tell that Hvad it's a library built by, and for, Swedish vikings. :)

I've started using django-i18n-model in my job, so I think I might contribute a fair bit more in the near future. Keep up the good work!

Comments (3)

  1. egasimus author

    Well, having to pass the actual translation model instead of the source model to inlines would seem like a backwards-incompatible change. And if anyone happens to be overriding I18nInlineMixin.set_extra, it's got an extra parameter now - matching the corresponding method in Django. Nothing else, I think.

  2. Branko Vukelic repo owner

    Ok... now I get it. Remind me to not review code while doing a completely unrelated project. :D

    Yeah, well 'backwards-incompatible' is something to worry about when code is mature and there are many users. Until 1.0 I guess it's fine to break things when it makes sense, like in this case. Passing the translation model indeed makes much more sense. Not sure where I got the idea to use the source model anyway. :D

    Anyway, give me a few days. Not at my home box atm and I have a project I want to test this one. I've looked at the code in more detail, and it all looks good.