Skip to content

Commit

Permalink
Refs #28814 -- Fixed "SyntaxError: Generator expression must be paren…
Browse files Browse the repository at this point in the history 
…thesized" on Python 3.7.

Due to https://bugs.python.org/issue32012.
  • Loading branch information
@timgraham
timgraham committed Nov 17, 2017
1 parent 648957b commit 931c60c
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions django/contrib/admin/widgets.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,7 @@ def get_context(self, name, value, attrs):

params = self.url_parameters()
if params:
related_url += '?' + '&'.join(
'%s=%s' % (k, v) for k, v in params.items(),
)
related_url += '?' + '&'.join('%s=%s' % (k, v) for k, v in params.items())
context['related_url'] = mark_safe(related_url)
context['link_title'] = _('Lookup')
# The JavaScript code looks for this class.
Expand Down

23 comments on commit 931c60c

@ppawlak
Copy link

@ppawlak ppawlak commented on 931c60c Jul 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this fix be merged in older releases of Django? 2.0 and/or 1.11?

@timgraham
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the FAQ, Django 2.0.x is compatible with Python 3.7.

@ppawlak
Copy link

@ppawlak ppawlak commented on 931c60c Jul 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll migrate my project to 2.0 then

@Eric727
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks,

@ronozoro
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply the fix for django 1.11

@C-REMO
Copy link

@C-REMO C-REMO commented on 931c60c Aug 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix works on Django 1.11 too. Thanks!

@Eric727
Copy link

@Eric727 Eric727 commented on 931c60c Aug 12, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kir12
Copy link

@kir12 kir12 commented on 931c60c Aug 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm running Django 1.11.15 (the latest version in 1.11) with Python 3.7 on Manjaro Linux (a derivative of Arch Linux), and I'm still getting the same error. Here's the steps I took to reproduce the problem:

sudo pacman -Syu
cd /path/to/root/project/directory
python -m venv myapp
source myapp/bin/activate
pip install Django==1.11.15
django-admin startproject mysite
cd mysite
python manage.py runserver

Updating Pip to the latest version and reinstalling Django didn't help either. Per this SO Question it would appear that the bugfix hasn't made it into fresh releases of Django 1.11. Is Django 1.11 getting this bugfix, or will I have to upgrade my project to v2.0?

@timgraham
Copy link
Member Author

@timgraham timgraham commented on 931c60c Aug 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the FAQ, Django 1.11.x is not compatible with Python 3.7.

Django 1.11.x reached end of mainstream support on December 2, 2017 and it receives only data loss and security fixes until its end of life.

@ociule
Copy link

@ociule ociule commented on 931c60c Sep 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timgraham Could this patch be bundled into the next data loss / security fix update please ? I understand shipping a new release is a lot of work, but bundling with one you'd make anyway ?

Django 1.11 is the LTS version, and it would be nice if it worked on the latest Python. Given Python does not have LTS versions, starting a project today on Django 1.11 LTS / Python 3.7 is a reasonable strategy to get maximum life time out of these two major components.

@timgraham
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no plans to add Python 3.7 support to Django 1.11. Look on the ticket for this commit and you'll see that more patches are required than just this one. If you wish to raise a discussion, use the django-developers mailing list.

@ociule
Copy link

@ociule ociule commented on 931c60c Sep 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timgraham Thanks a lot for the info, that makes sense. Yeah, sounds like upgrading is the best way if there are multiple compatibility patches.

@banjin
Copy link

@banjin banjin commented on 931c60c Sep 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以将这个修改直接应用到django1.11和python3.7环境中

@karolyi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timgraham it may be only me, but after applying this one change to my django1.11.16, it started up without any complaint.

@karolyi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, here's a thing that's not resolved at this moment: information here tells that this patch is not gonna be applied to the LTS django because of other outstanding issues.

however, 2.x does not have an LTS version yet.

moreover (and I know that this is not django's fault), the django-modeltranslation module also doesn't support django2.x yet, see deschler/django-modeltranslation#472 and deschler/django-modeltranslation#467). this makes us, poor django peasants stick with python3.6.

since I don't see what other issues apply to the LTS version with 3.7, I consider it dangerous to use in production, so I have to revert back to using 3.6. no data classes for me I guess.

really looking forward to have this conflict resolved.

@carltongibson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @karolyi.

This won't be backported due to Django's supported versions policy. (Not any other outstanding issues.) The place to take up a discussion of the policy is the django-developers mailing list, not here.

And 💃🏼, dataclasses was backported to v3.6 if you're missing it: https://pypi.org/project/dataclasses/

really looking forward to have this conflict resolved.

TBH, there's not really any chance of that. It's just unfortunate timing that Python 3.7 came after Django 1.11. (If you're on an LTS, you have to expect not to be on the bleeding edge: it can't work both ways.)

@karolyi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carltongibson, thx I didn't know about dataclasses existing for 3.6. I'll look into it.

OTOH, is 3.7 'bleeding-edge' though? it's considered stable, so it would be nice to be able to use it with an LTS version of django. I have a project with lots of dependencies and compiled stuff (from xapian to pillow, QR code generators and whatever), everything went a-okay except for this one error.

I agree, the timing was unfortunate. I'm not complaining though, don't get me wrong. I'm just stating the facts.

@carltongibson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is 3.7 'bleeding-edge' though?

Maybe bad choice of words but, from the perspective of Django 1.11, Yes, Python 3.7 is bleeding-edge: Django 1.11 ended mainstream support in December 2017. Python 3.7 wasn't released until June 2018. Thus it's not supported.

The next LTS will be April 2019. It'll support Python 3.7, but by the time it's EOL no-doubt they'll be new Python versions it doesn't support. Again, that's kind-of entailed by choosing the LTS. If you want the latest choose different...

@zyv
Copy link
Contributor

@zyv zyv commented on 931c60c Nov 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

TLDR; I'm happy to make a PR if anyone from core team would be willing to consider it (/cc @timgraham).

In our view, after Python development has picked up so much speed and stable official releases are coming out so often, 3-year LTS support policy excluding support for newer runtimes warrants reconsideration, especially if the changes are only so few, non-risky and so trivial.

I have backported all relevant commits that I could find to Django 1.11.16 LTS:

https://github.com/moneymeets/django/commits/moneymeets/1.11.16-py37

We are currently testing the resulting artefact, but the Django test suite passes and everything seems to work nicely so far.

You can use our build if you specify

--extra-index-url https://moneymeets.github.io/django/

in your requirements.txt or

[[source]]
url = "https://moneymeets.github.io/django/"
verify_ssl = true
name = "moneymeets"

if you use pipenv.

Note that for various reasons we had to hack the versioning scheme, so Python 3.7 compatible releases are numbered 1.11.161 (based upon 1.11.16), 1.11.171, etc.

The code is provided as is without any warranties and/or support, it is not an official Django offering, be sure you understand what you are doing and be prepared to keep the pieces if something breaks.

@karolyi
Copy link
Contributor

@karolyi karolyi commented on 931c60c Nov 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zyv wow, I'd be happy to see this as 1.11.17!

I also use 1.11.16 with python3.7 and I have to edit that one line manually each time to get it working.

@timgraham
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened a thread on django-developers about adding Python 3.7 support to Django 1.11.x.

@timgraham
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yury, create a pull request with your branch so we can see if all tests pass there.

@zyv
Copy link
Contributor

@zyv zyv commented on 931c60c Nov 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Tim, done in #10654; thank you for being open minded about this and for all your work on Django, irrespectively of the outcome of this thread!

Please sign in to comment.