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

[WIP] Python3 compatibility #156

Closed
wants to merge 21 commits into from
Closed

Conversation

AdamIsrael
Copy link

@AdamIsrael AdamIsrael commented Feb 16, 2018

Building off of the work of @dbarrosop, this pull request is focusing strictly on the changes required to introduce Python3 compatibility. In doing so, I've dropped some of the initial changes (such as the use of OrderedDicts to guarantee the order of dictionary results) that are better proposed in a separate pull request, if someone feels strongly about those changes.

Outstanding items to address:

  • Fix test - serialise/ietf-json-deserialise
  • Fix test - serialise/json-deserialise
  • Fix test - serialise/openconfig-serialise (also broken in master)
  • Fix test - integration/openconfig-interfaces
  • Confirm all tests pass with Python3

dbarrosop and others added 19 commits August 19, 2017 11:21
py3 re stdlib fails if the  regular expression contains things it can't understand like \p. However, regex actually understands those constructs
(Pdb) bitarray.bitarray('010101')
bitarray('010101')
(Pdb) bitarray.bitarray(u'010101')
bitarray('111111')
Based on the work done by @dparrosop in pull request robshakir#138, I have done
some refactoring to limit the scope of work to only changes addressing
Python3 compatibility.

Changes unrelated to Python3, such as the use of OrderedDict(), have
been removed for the sake of clarity and should be addressed in a
separate pull request by anyone who feels strongly about that behavior.

Changelog:
- Removing usage of OrderedDict(), not required for py3 compatibility
- Merge serialise.py from upstream with patch for default values
- Removed check of base_type == bitarray in yangtypes.py; it's unclear
what this change was intended to do.
- Removing trailing whitespace (editor quirk)
- Fix bash-related execution problem in tox.ini (thanks to @ktbyers)
- Revert peer_groups = 0 to false in test json
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

I left a line commented out that prevented this test from running.
@dbarrosop
Copy link
Contributor

I've dropped some of the initial changes (such as the use of OrderedDicts to guarantee the order of dictionary results) that are better proposed in a separate pull request

Sounds good to me, will send a new PR with only that soon. Let me know if you removed something else as well, please.

@robshakir
Copy link
Owner

On the CLA, the other submitter is @dbarrosop - so we're all good on CLA coverage. Thanks to both of you.

@tarkatronic
Copy link
Collaborator

@AdamIsrael As I previously mentioned, I'd love to help out with this effort. For easy coordination of efforts, might I suggest the Network To Code Slack instance?

Also, @robshakir, I can't seem to find the CLA for this project. Any pointers there?

@robshakir
Copy link
Owner

@tarkatronic The CLA is the Google CLA that can be signed at https://cla.developers.google.com/clas

Just as background -- when I moved to Google, we wanted to make sure that the contributions to pyangbind were such that they would allow free use of the code under the Apache 2.0 license, with clarity around copyright and contributions. My initial post on the subject was here. If there are any questions on this, please do let me know and I can either answer, or I can ask our OSS team here to help out.

I'm happy to discuss on the Network To Code Slack, do you want to create a channel there?

@tarkatronic
Copy link
Collaborator

@robshakir Great! I've created #pyangbind on Slack, and I'm working on getting added to GoDaddy's CCLA for Google.

Handle the dictionary change between py2/3
@tarkatronic
Copy link
Collaborator

@AdamIsrael Thank you for the help in getting Python 3 compatibility! Closing this up now, as we've gotten the work completed in master now.

@tarkatronic tarkatronic closed this May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants