You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@tinkerpop.apache.org by davebshow <gi...@git.apache.org> on 2016/09/14 16:48:40 UTC

[GitHub] tinkerpop pull request #415: Tinkerpop 1448: gremlin-python should be Python...

GitHub user davebshow opened a pull request:

https://github.com/apache/tinkerpop/pull/415

Tinkerpop 1448: gremlin-python should be Python 2/3 compatible

https://issues.apache.org/jira/browse/TINKERPOP-1448

This PR adds code to promote Python 2/3 compatibility in the gremlin-python GLV. The main change here involves adding a module, `compat`, that provides 2/3 compatible type info.

`compat` defines a class `long` for Python 3. `long` inherits from `int` and behaves like an integer. The advantage of creating this class is it allows application developers to type an integer as `long` in Python 3 code, which will result in serialization as `int64` in GraphSON.

`compat` also provides values for standard library `types` module that are not included in Python 3 (`IntType`, `FloatType`, and `LongType`).

Finally, this PR uses `six.get_function_code` for introspection, as this API changed with Python 3.

While all tests have been updated accordingly, when building with `mvn clean install -pl gremlin-python -DglvPython` tests are only run against the local machine's default version of Python. Therefore, tests are not run on both versions by default, however, they *will* now pass if a users default Python interpreter is Python 3.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-1448

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/415.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #415

----

---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #415: Tinkerpop 1448: gremlin-python should be Python 2/3 co...

Posted by spmallette <gi...@git.apache.org>.

Github user spmallette commented on the issue:

    https://github.com/apache/tinkerpop/pull/415
  
    @davebshow what are the risks of building python 2 and not 3 or the other way around? will we easily break compatibility such that not building against both on a constant basis will cause problems? or can building both be saved for "integration tests" which run less frequently? 
    
    put in more context, is it worth extending the build time of `mvn clean install` to cover both python 2 and 3? 
    
    I'm sorta hoping the answer is "no" and that we can make running "both" part of integration tests only. 
    
    Also, what does python 2/3 compatibility mean for deployment? does anything change in how this stuff goes up to pypi?
    
    On the whole, this is most excellent btw. We really needed this. I do think that we should try to figure out how all the test/deploy stuff gets settled before merging though. I suggest we further wait for #416 to merge to master so that you can rebase this branch. Then I can work with you to tweak the pom.xml from there based on whatever is settled on for test/deploy issues.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #415: TINKERPOP-1448 gremlin-python should be Python 2/3 com...

Posted by davebshow <gi...@git.apache.org>.

Github user davebshow commented on the issue:

    https://github.com/apache/tinkerpop/pull/415
  
    1c5e698 add simple package metadata.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #415: Tinkerpop 1448: gremlin-python should be Python 2/3 co...

Posted by davebshow <gi...@git.apache.org>.

Github user davebshow commented on the issue:

    https://github.com/apache/tinkerpop/pull/415
  
    Maintaining compatibility should be pretty simple, but as you can see from the PR, there are little things that can go wrong. I think the main issue we will run into with this library is numeric typing, and I think we are well on our way to dealing with it.
    
    Regarding the mvn build, I think we can just have it run against the default Python installation for now, which is a bit more risky, but we can mitigate the risk by running CI builds for multiple versions of Python. I also like to test locally, I just spin up a server manually and run `python setup.py test` against multiple versions of Python. This kind of multiple environment testing is made easier using the tool `tox`, maybe we could look at adding this as a requirement.
    
    This shouldn't affect the deployment process at all. The only difference is now we can add Python 3 to the package metadata.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---