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

Make it work with Python 2.7 #66

Closed
djc opened this issue Mar 16, 2015 · 15 comments
Closed

Make it work with Python 2.7 #66

djc opened this issue Mar 16, 2015 · 15 comments
Milestone

Comments

@djc
Copy link

djc commented Mar 16, 2015

Would you accept a pull request to make this work with Python 2.7 (from a single-source code base)? If so, how much work do you think it would be?

@scopatz
Copy link
Member

scopatz commented Mar 16, 2015

Yes, I would happily accept such a pull request. I think it would be a fair amount of work (but not nearly as much as it was to start from scratch), most of which is in the parser. I'll outline the steps later today, after work.

@scopatz scopatz added this to the v0.2 milestone Mar 16, 2015
@scopatz
Copy link
Member

scopatz commented Mar 17, 2015

Hi @djc - Would be delighted to see this happen. To do this, I would do the following:

  1. Start by looking at the official Python 2.7 language grammar (https://docs.python.org/2/reference/grammar.html). You may also want to look at the v3.4 grammar, just to see what is different.
  2. Pick a single feature (like the print statement).
  3. Find the relevant line in the grammar that refers to this feature.
  4. Translate the line in the grammar to the PLY grammar format. Note that they Python grammar uses a lot of [optional] syntax that PLY does not support, so you have to expand optional to their own lines.
  5. Make a parser method that implements the new grammar.
  6. Add a test for the new feature.
  7. Make sure all of the tests pass or are skipped based on the version, if Python version dependent.
  8. Goto (2) until v2.7 compliant.

Note that the parser produces an AST using Python's own AST node. You may need to adjust which nodes xonsh.ast imports based on the version. I don't think that there is much more too it than that. You may need to add a PRINT token to the lexer, but that should be all. Turning on debugging mode and testing each test individually helps when you know you are working on a particular piece of syntax. This process can be a little tedious. But once it is done, the hard work is over :).

Don't hesitate to ask questions, ask me to look at a branch, or anything. I am more than happy to help. The PLY documentation is pretty great and there are some nice tutorials out there too. I hope this helps.

@ludovic-gasc
Copy link

From my point of view, Python 2 compatibility will complexify the source code with a big risk to have more bugs, for what advantage ?
xonsh isn't a Python library: it's a tool to be used on a developer laptop. To my knowledge, all major Linux distributions already have Python 3.4 included and no problems on Windows to install Python 3.4.

@oersted
Copy link

oersted commented Mar 30, 2015

Actually, Debian Wheezy (Stable) is still using Python 3.2 and it's a bit of a pain to install 3.4 as it is necessary to add the Sid repositories, which shouldn't be done generally. A manual installation is always an option, but I guess there's a reason why 3.4 isn't yet in the stable repositories. If you want to use pip to install Xonsh, pip3.4 has to be installed too. Taking into account how much people uses Debian, it wouldn't be a bad idea to support more Python versions. Adding that compatibility to the existing tool would indeed add a lot of complexity. Maybe it would be a good idea to make the Python 2.7 based one an independent tool, or at least isolated from the current parser. It doesn't make sense to support both at the same time.

@scopatz
Copy link
Member

scopatz commented Mar 30, 2015

Hi @oersted, I didn't know debian was still on v3.2. I am happy to support older versions of Python, including the 3 series. But this is a place where we could use some serious help, independent of how it is implemented. If someone iworking on this and running into problems, please let us know!

@ludovic-gasc
Copy link

@oersted: You can install easily Python 3.4 on Debian Wheezy with Pythonz project: https://github.com/saghul/pythonz

BTW, it isn't my personal time, but if I was a developer of xonsh, I would work to implement new features instead of to try to support all Python versions.

@guy-do-or-die
Copy link

is there anybody working on that? i would love to participate if someone more experienced can lead the process.

@scopatz
Copy link
Member

scopatz commented Mar 14, 2016

Hey @guy-do-or-die, I don't believe that anyone is actively working on this, so please feel free! The steps are basically the same as listed in my previous comment.

The first major difference though is that you should check out the xonsh.parsers sub-package and create a v27 parser that works on the Python 2.7 language. You should subclass the base parser to do this. You may want to diff the language grammar from v2.7 to v3.4 to see what the changes were. Let me know if you need any more help getting started.

@funkyfuture
Copy link
Contributor

what would be the rationale behind this? that certain OS still lack support for Python 3?

if it works, packaging with pyinstaller may be a viable option to deal with it.

@funkyfuture
Copy link
Contributor

i think this question should be decided rather sooner than later as it affects the leverage when implementing features.

also, atm i'm itchy to remove code fragments that are meant for Python 2.

beside the points that @GMLudo made, i'd throw in for consideration that supporting Python 2 will make external dependencies necessary as vox depends on venv. generally Python 3 code can be way simpler which imo is a benefit for rather complex applications.

@gforsyth
Copy link
Collaborator

Given that we've had problems even supporting Python 3.4 on occasion, I do think it would be a... large... undertaking to port xonsh back to Python 2.
I would like to see vox moved out of xonsh main and put in as a xontrib, where I think it fits better.

It would be @scopatz decision in the end, but I would encourage any potential Python 2 port to be maintained as a separate fork, as the effort of adding new features that are also backwards compatible with legacy Python can be a real pain.

@melund
Copy link
Member

melund commented May 14, 2016

I agree. If you are thinking of contributing you can safely write Python 3 code. That said no one would be against someone trying to port it to Python 2.7.

@scopatz
Copy link
Member

scopatz commented May 26, 2016

Hey all, sorry this got lost in my email. @funkyfuture - I think that this issue was made at a time when it was much less certain that everyone was going to move to Python 3 eventually. I actually highly doubt that anyone would be able to just drop a full Python 2 port on us overnight and have it work. The code base is evolving too quickly for that. That said, if this did magically happen tomorrow, I think it would be hard to not merge it.

In any event, I certainly don't want to maintain Python 2 code past 2020, or better yet, past 2019, or even tomorrow :). I experience a weird dissonance now when I look at Python 2 code and think, "how did I ever like this!"

It is probably fine to keep this issue open, until we are sure it won't happen.

@gforsyth, as per vox, I totally agree. Vox should be a xontrib. Probably a xontrib that comes with xonsh, like mpl, but still. It shouldn't be loaded as an alias every time for all users. I never use venvs, personally. Maybe we should make another issue for that.

@gforsyth gforsyth modified the milestones: v0.3, v0.4.0 Jun 5, 2016
@mitnk
Copy link
Member

mitnk commented Jun 6, 2016

I tend to not support Python 2, if we have to make a trade off.

@scopatz
Copy link
Member

scopatz commented Aug 17, 2016

Given #1560 and the discussion leading up to it, I think it is a fair move to close this for the time being. If some tuxedo mask feels like dropping a 2.7 PR on us, please do! Though that seems increasingly unlikely...

@scopatz scopatz closed this as completed Aug 17, 2016
@xonsh xonsh deleted a comment from ZhouShisong Nov 10, 2017
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

9 participants