Added deprecation warning to NLBankAccountNumberField

[jieter]

ref #150

All Changes

• Add an entry to the docs/changelog.rst describing the change.

• Add an entry for your name in the docs/authors.rst file if it's not
  already there.

• Adjust your imports to a standard form by running this command:

  `isort --recursive --line-width 120 localflavor tests`
  

[codecov-io]

Codecov Report

Merging #307 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #307      +/-   ##
==========================================
+ Coverage   96.26%   96.26%   +<.01%     
==========================================
  Files         156      156              
  Lines        4360     4361       +1     
  Branches      588      588              
==========================================
+ Hits         4197     4198       +1     
  Misses        102      102              
  Partials       61       61

Impacted Files	Coverage Δ
localflavor/nl/models.py	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f04d55...eb8e8b6. Read the comment docs.

[benkonrath]

Thanks for this. I've added some comments for some improvements.

[benkonrath]

For deprecating the phone number models we're using the system check framework. Do you think it make sense to use the system check framework here as well?
https://github.com/django/django-localflavor/blob/master/localflavor/deprecation.py#L21-L24

Also, IBANField isn't a direct replacement for NLBankAccountNumberField. Users would need to do a data migration and generate the IBAN for each NL bank account number. My feeling is that adding this data migration is out of scope for localflavor. Calculating the check digits straightforward but we'd need to somehow map the NL account numbers to a 4 character IBAN bank ID and I don't this mapping readily available. I think we should just have a note stating that a data migration is required if users want to directly replace the NLBankAccountNumberField with the IBANFeild. Otherwise, user will need to use both fields in parallel for a while.

[jieter]

I have no preference regarding the systems check framework vs using warnings.warn. If you do prefer using the systems check framework, I'll be happy to change the patch.

You're right about the data migration, I'll add a note.

[benkonrath]

I prefer the systems check framework to keep things consistent with the other model deprecations. Thanks.

[jieter]

@benkonrath, holidays came in the way, I'll try to work on this coming week.

[jieter]

@benkonrath changed the deprecation warning and also rebased on current master.

[benkonrath]

Looks good. You just need to remove the unused import.

localflavor/nl/models.py
  Line: 6
    pyflakes: F401 / 'localflavor.deprecation.RemovedInLocalflavor20Warning' imported but unused (col 1)