Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating the source to Python 3? #208

Closed
emmeowzing opened this issue Aug 19, 2017 · 16 comments
Closed

Updating the source to Python 3? #208

emmeowzing opened this issue Aug 19, 2017 · 16 comments

Comments

@emmeowzing
Copy link

Just a quick comment--looking over the source, do you think it'd be a good idea to update to Python 3, e.g. using type annotations and Python 3 syntax?

@pylang
Copy link

pylang commented Aug 19, 2017

+1 Python 2/3 compliant code
-1 Annotations (I don't think it's necessary at the moment)

@faif
Copy link
Owner

faif commented Aug 20, 2017

I second @pylang. Python 2/3 compatibility is more important than type annotations for this project

@brettswift
Copy link

While we're on the topic of python 3, I am finding a difference in behaviour running the registry example.
https://github.com/faif/python-patterns/blob/master/behavioral/registry.py

With python 3.6 I do not see the ClassRegistree printed. Copied the code verbatim and ran it.

@egens
Copy link
Contributor

egens commented Dec 7, 2017

In python 3 metaclass is added differently

class BaseRegisteredClass(object, metaclass=RegistryHolder):

instead of

class BaseRegisteredClass(object):
    __metaclass__ = RegistryHolder

https://stackoverflow.com/questions/5189232/how-to-auto-register-a-class-when-its-defined

@gyermolenko
Copy link
Contributor

More than a year since last comment here. Maybe it's time to remember about the topic? :)

Considering python2's EOL I would like to see python3-only code.
-1 for type annotations.

@fkromer
Copy link
Contributor

fkromer commented Jan 9, 2019

There are 11 months left until Python2 EOL 😉 keeping the Python2/3 compatiblility is probably helpful for some people still working with Python2 code. But a Python3 branch would be nice.

-1 for type annotations.

@faif
Copy link
Owner

faif commented Jan 11, 2019

Agree with @fkromer Let's revisit this after the Python 2 EOL

@gyermolenko
Copy link
Contributor

gyermolenko commented Feb 11, 2019

There are a couple of examples with __metaclass__ syntax (registry, chain of responsibility, some more..).

  1. Syntax for Python 2 and 3 is different
  2. Being run with different python versions scripts produce different output .
  3. There is no way to write them in compatible way.
    I don't like workaround with six lib because
    • any dependency is unwanted
    • examples will lose both Python2 and Python3 look:
from six import with_metaclass

class MyClass(with_metaclass(Meta)):
    pass

Proposition:

  1. add note to README. smth like
DISCLAIMER
Although we try to keep 2/3 compatibility most outputs are given for Python3 by default. 
In some situations (e.g. when metaclasses are being used) there is no compatible syntax. This is why such scripts might fail with SyntaxError or give unexpected output for Python2.
Such scripts will have compatibility DISCLAIMER on the top.
  1. Update scripts to use Python3 only metaclass syntax and add DISCLAIMER on the top

This is different from "Update everything to Python3 syntax, use f-strings, etc etc ".
It is rather "Python 3 first, compatible with Python2 syntax as much as possible". In case of metaclasses it is just not possible (py2 syntax doesn't work in py3, wo errors. py3 syntax raises SyntaxError in py2)

@faif
Copy link
Owner

faif commented Feb 12, 2019

@gyermolenko How about using six until the Python 2 EOL? After that, we can get rid of it and keep only the Python 3 functionality

@gyermolenko
Copy link
Contributor

gyermolenko commented Feb 13, 2019

My issue with six is that code becomes neither py2 nor py3.
So whatever user is looking for, if he doesn't use six himself already, - he will need to modify script.

Examples below:

class Meta(type):
    pass

# Python 2 only:
class MyClass(Meta):
    __metaclass__ = Meta
    pass

# Python 3 only:
class MyClass(metaclass=Meta):
    pass

# with six
from six import with_metaclass

class MyClass(with_metaclass(Meta)):
    pass

ps. I am pro Python3-first, not Python3-only approach.
And so far only examples with metaclasses are controversial (that is 3-5 scripts).

@faif
Copy link
Owner

faif commented Feb 13, 2019

The maybe it's better to split those files to Python 2/3 versions until the Python 2 EOL

@gyermolenko
Copy link
Contributor

gyermolenko commented Oct 29, 2019

hey @faif
Not much time left till EOL.
I would like to clean some py2 specific things up if you don't mind.

So maybe it makes sense to create git tag or release and proceed with py3-only version in master?
Other options (which I don't like) would be:

  • proceed with maintaining 2/3 compatible code
  • maintain separate branches for 2 and 3

And with git tag later someone can switch from github ui or do git checkout past versions easily.

Please share your thoughts so I prepare changes accordingly.

@faif
Copy link
Owner

faif commented Oct 29, 2019

I like the tag solution. That way anyone still interested in the Python 2 solutions can access them. Let’s cleanup master to use Python 3 only and have a reference to the tag.

@gyermolenko
Copy link
Contributor

I think only you can push new tags.

@faif
Copy link
Owner

faif commented Oct 29, 2019

@gyermolenko I pushed a legacy tag that contains the current repo contents. Feel free to start cleaning up 👍

@gyermolenko
Copy link
Contributor

the issue can be closed

@faif faif closed this as completed Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants