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

Python 3 support #57

Open
anlutro opened this issue Dec 17, 2016 · 8 comments
Open

Python 3 support #57

anlutro opened this issue Dec 17, 2016 · 8 comments

Comments

@anlutro
Copy link

anlutro commented Dec 17, 2016

Hey,

I'm interested in integrating reclass into an existing Python3 library, but there are some incompatibilities. Most of them seem rather easy to fix, like changing print to a function call (with from __future__ imports), using a leading dot for relative imports, and except Exception as e.

Is this something you've considered doing? Would you like a pull request for it?

@Rtzq0
Copy link
Collaborator

Rtzq0 commented Dec 27, 2016

I think mixed 2 and 3 builds is definitely on the long-term-unofficial wishlist. I promise that if you submit a PR it will get reviewed in a timely fashion!

@anlutro
Copy link
Author

anlutro commented Dec 29, 2016

PR coming. There's one place where a non-trivial change has to be made - Parameters.interpolate uses .iteritems().next() and some weirdness where it removes items from the dictionary while iterating.

I replaced it with this and no tests failed but might have unintentional side effects.

for path, refvalue in self._occurrences.copy().items():
    self._interpolate_inner(path, refvalue)

@lottspot
Copy link
Collaborator

@anlutro you should be able to use something like

path, refvalue = iter(self._occurrences.items()).next()

In order to get the same result which is both py2/3 compatible

@Rtzq0
Copy link
Collaborator

Rtzq0 commented Dec 29, 2016

I was planning to discuss this more in the review for #58 , the other question that I had with that change was the removal of:

while self.has_unresolved_refs():

at 175. That's why I commented I didn't understand the 2->3 move here.

@Rtzq0
Copy link
Collaborator

Rtzq0 commented Jan 6, 2017

@lottspot since this is a general question, I guess I'll put it here instead of in the PR

I think we're in agreement that alterations to python 2 behavior should be considered out of scope for this unless utterly required. A bigger question is "should we use a compat library to avoid writing our own wrappers and if so should it be six or future?"

For the record I lean toward future as it's cleaner and has larger coverage, and I don't really care about python 2.4 and 2.5 or 3.2 (RHEL/CENTOS 5 is EOL now)

@Rtzq0 Rtzq0 added this to the 1.4.2 milestone Jan 6, 2017
@lottspot
Copy link
Collaborator

lottspot commented Jan 6, 2017

I'm not particularly in favor of introducing an additional dependency in order to have py2/3 compatibility. Writing backwards compatible python3 is a trivial task in most cases, and I would want to see something in the code base which is not trivial to rewrite in a backwards compatible way before adding a new dependency to the project.

@Rtzq0
Copy link
Collaborator

Rtzq0 commented Jan 6, 2017

Alright then that pretty much sets 2 requirements for the resolution to this issue:

  1. Any changes to python 2 functionality must pass a high bar of necessity (and must be justified)
  2. Additional dependencies are out of scope

Are you in agreement with these limits?

@lottspot
Copy link
Collaborator

lottspot commented Jan 6, 2017

I would agree with that. I think we could loosen up requirement 2 even to be more along the lines of number 1 in that introducing a new dependency doesn't necessarily have to be out of scope, but an exceptional need should be demonstrated before we accept the introduction of such a dependency

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

3 participants