Port to Python 3

Issue #404 resolved
Ed McDonagh created an issue

Need to port to Python 3 this year (2019) - support for 2.7 drops at the start of 2020.

No longer any blocking dependencies - django-pagination no longer being used, django-qsstats-magic has been upgraded to support Python 3, and presumably django-solo has too.

No point attempting to support Python 2.7 and Python 3 simultaneously - all users will need to go through the pain of the upgrade, we may as well go through it together! Also, targeting Django 2.2 which only supports Python 3.5 to 3.7.


Comments (104)

  1. Ed McDonagh reporter

    Extractor files have remapp/Django setup stuff at top, doesn't need to be duplicated, doesn't need to import file that was upsetting futurize. Refs #404

    → <<cset 1a4b694e72ad>>

  2. Ed McDonagh reporter

    Disabling pylint warnings regarding overriding builtins due to future. Refs #404 Should be possible with a single config line in pylint, but I'm not sure how to do it in Codacy!

    → <<cset 827ba5240239>>

  3. Ed McDonagh reporter

    @dplatten @LuukO - can you take a quick look at http://testing.openrem.org/openrem/ct/ please?

    I've replaced the django-pagination function with the in-built Django pagination function, plus some styling from a gist I came across.

    What do you think? We need to stop using django-pagination to move onto newer versions of Django and Python. And it isn't required anymore anyway!

    Also, David, the charts on that page are blank for me - am I doing something wrong?

  4. Ed McDonagh reporter

    I think I'll probably reduce the number of 'neighbours' down from the current 10 to about 5.

  5. Ed McDonagh reporter

    @dplatten - Both displaynameview.html and rfalertnotificationsview.html have {% paginate %} tags in them, but they don't have the {% autopaginate object_list %} tags in them, so I don't think the pagination has ever functioned. I can't see how it would function for the display names page!

    Question - do I just remove the tags (I think functionality stays as is), or do I fix them up, or maybe just the RF Alert Notifications page so that they paginate using the new method?

  6. Ed McDonagh reporter

    Added paginate to filteredbase template, removed old paginate from templates, added to views for dx, rf, mg. Reformatted filteredbase template.. Refs #404

    → <<cset 48840a72ac1e>>

  7. Ed McDonagh reporter

    Broke the home page as AnonymousUser has no user profile attribute. Elsewhere the user will be logged in, so should be ok... I think! Refs #404

    → <<cset c50ace87ed72>>

  8. Ed McDonagh reporter

    @dplatten I have found that if 'plot a series per system' is ticked, the charts are empty. They are fine otherwise.

    Have I broken something?

  9. David Platten

    @edmcdonagh, I've been looking at the same thing (in the PHE CT export branch). I have fixed the problem by changing a couple of lines of code in the charting functions. I'm not sure what has caused the break.

  10. Ed McDonagh reporter
    • changed milestone to 1.0.0

    Setting milestone to 1.0.0. Need to crack on with this now for the next release after 0.10

  11. Ed McDonagh reporter
    • edited description

    Updated Description to indicate that we can’t support Python 2 and 3 simultaneously as we’ll be targeting Django 2.2.

  12. Ed McDonagh reporter

    Filter views now display, f.queryset instead of f in paginator, but filter doesn't display. MethodFilter replaced with CharFilter(method=. Refs #404, #437 [skip ci] not ready for tests

    → <<cset debb9f7bb725>>

  13. Ed McDonagh reporter

    Starting to reinstate and update DXSummaryListFilter. Age gt not currently working. Might continue upgrade to later Django and version 2 of django_filter before attempting to fix. Refs #404, #437 [skip ci] not ready for tests

    → <<cset 8bc5408c2776>>

  14. Ed McDonagh reporter

    DX summary filtered page now working with all filters except age with django_filters 1.1, Python 3.6, Django 1.8. Need to repeat for other modalities, then move onto Django 1.9 I think. Refs #437, #404 [skip ci] not ready for testing

    → <<cset 05dc823484a9>>

  15. Ed McDonagh reporter

    Commented out requirements so OpenREM could be 'installed' in this env without old list of packages. Refs #404 [skip ci] - not ready for testing

    → <<cset ab8a179c68f5>>

  16. Ed McDonagh reporter

    Code meaning now needs to be explicit for some reason, plus other Python 2/3 string related changes. mammo csv export tests now work. Refs #404 [skip ci] - not ready for testing

    → <<cset e7b442b522f5>>

  17. Ed McDonagh reporter

    'UN' VRs are now read in as byte strings, so need decoding before conversion to Decimal. Must be better method with newer pydicom. Refs #404 [skip ci] - not ready for testing

    → <<cset 54cc4ca7594e>>

  18. Ed McDonagh reporter

    Not sure why some mam image values are bytes that need decoding before casting to Decimal, others are float or int. Hence try:except:. Also corrected mod_filters test and reverse_lazy. refs #404, #437 [skip ci] - not ready for testing

    → <<cset cdf71f122dc2>>

  19. Ed McDonagh reporter

    Appears that names read from DICOM are byte strings, so reflecting that in test. Need to investigate more thoroughly with pydicom. Refs #404, #777 [skip ci] docs only.

    → <<cset 2f8c35cd6eda>>

  20. Ed McDonagh reporter

    Need to encode unicode strings before using hashlib, but fixed as hash strings from Python 2 so we know pre/post upgrade hashes will match. Refs #404 [skip ci] docs only.

    → <<cset 21a7340f9f9d>>

  21. Ed McDonagh reporter

    Tests find the root by going up until no init.py is found. There shouldn't be an init.py at the same level as manage.py. Refs #404 [skip ci] Not ready for testing

    → <<cset fbcd0f28566e>>

  22. Ed McDonagh reporter

    Disabled pynetdicom testing for now. All other tests pass. Need to work out Python 3 testing rig on bitbucket. Refs #404, #777, #457, #530 [skip ci] Not ready for testing. Next task - move to Django 1.9...

    → <<cset e13fda4c134e>>

  23. Ed McDonagh reporter

    Pipeline failed during coverage due to 025 that looked like an invalid (for Py3) octal. Sourcecode suggests it should be 25 int. Also stopped imports from complaining. Refs #404

    → <<cset df0a0a3fe8ba>>

  24. Ed McDonagh reporter

    Changed everything from dicom to pydicom. Attempted to remove all byte warnings - elements with VR UN are imported as bytes. Existing db data might be bytes too. Refs #457, #404, #777

    → <<cset 6bd11d6314da>>

  25. Luuk

    Great work @Ed McDonagh ! I’ll try to get it running in my test (pre-production) environment.

    Just a remark to have it noted somewhere.

    Python 3.7 introduces the reserved word async (https://docs.python.org/3/whatsnew/3.7.html). This is a breaking change and will break celery 3 as the underlying kombu library uses the word “async” as a package. This is fixed in newer versions used by celery 4, but celery 4 is not supporting Windows anymore. So we need to rethink eventually: or to keep using celery or to support Windows (both at the same time will eventually become impossible). For now: stick to Python 3.6

  26. Ed McDonagh reporter

    Thanks Luuk. That’s really frustrating! Are there any alternatives for Windows?

    Would you mind creating an issue to track how we handle this?

  27. Ed McDonagh reporter

    Actually Luuk, don’t worry, I’ll do it. There are a couple of things I want to note!

  28. Log in to comment