You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/04/27 15:46:20 UTC

[GitHub] [iceberg] samredai opened a new pull request, #4650: Solution for InterpreterNotFound errors from tox

samredai opened a new pull request, #4650:
URL: https://github.com/apache/iceberg/pull/4650

   It's expected that developers won't conveniently have python 3.7, 3.8 and 3.9 installed for tox to use for tests so I've added a recommendation to use pyenv to the python README. This is what I do locally and it works nicely. The content here is definitely more for the CONTRIBUTING.md file but I think it's ok here in the readme during the refactoring.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] samredai commented on a diff in pull request #4650: Solution for InterpreterNotFound errors from tox

Posted by GitBox <gi...@apache.org>.
samredai commented on code in PR #4650:
URL: https://github.com/apache/iceberg/pull/4650#discussion_r860298644


##########
python/README.md:
##########
@@ -35,9 +35,22 @@ pip install -e .
 Testing is done using tox. The config can be found in `tox.ini` within the python directory of the iceberg project.
 
 ```
-# simply run tox within the python dir
 tox
 ```
 
+## Solution for `InterpreterNotFound` Errors
+
+Currently, tests run against python `3.7.12`, `3.8.12`, and `3.9.10`. It's recommended to install and manage multiple interpreters using [pyenv](https://github.com/pyenv/pyenv).
+```
+pyenv install 3.7.12
+pyenv install 3.8.12
+pyenv install 3.9.10

Review Comment:
   Agreed. It would definitely be safe to just install the latest patch version but unfortunately pyenv doesn't allow excluding it such as `3.8` or `3.8.*`. They explain some of the rationale in [this](https://github.com/pyenv/pyenv/issues/1946#issuecomment-847735677) issue.
   
   One bug (I think?) in pyenv is that if you have multiple versions inside the `python-version` file, it doesn't know how to pick them all up when you run the shorthand `pyenv install` command. It's interesting though because pyenv creates that file on its own.
   
   To be clear about what I mean, if you run `pyenv local 3.9.10` it will create a `.python-version` file that looks like this:
   ```
   3.9.10
   ```
   and then someone else can come along and just run `pyenv install` in that directory and it will install `3.9.10`.
   
   However, if you do `pyenv local 3.7.12 3.8.12 3.9.10`, it creates a `.python-version` file that looks like ours:
   ```
   3.7.12
   3.8.12
   3.9.10
   ```
   but running `pyenv install` doesn't work:
   ```sh
   $ pyenv install
   python-build: definition not found: 3.7.12
   3.8.12
   3.9.10
   
   The following versions contain `3.7.12
   3.8.12
   3.9.10' in the name:
     3.7.12
     3.8.12
     3.9.10
   
   See all available versions with `pyenv install --list'.
   
   If the version you need is missing, try upgrading pyenv:
   
     brew update && brew upgrade pyenv
   ```
   
   If it's a simple fix in [pyenv-install](https://github.com/pyenv/pyenv/blob/master/plugins/python-build/bin/pyenv-install) to get it to iterate and install each version, then it might be worth trying a PR there.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on pull request #4650: Solution for InterpreterNotFound errors from tox

Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4650:
URL: https://github.com/apache/iceberg/pull/4650#issuecomment-1111495731

   Thanks @samredai!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] dramaticlly commented on a diff in pull request #4650: Solution for InterpreterNotFound errors from tox

Posted by GitBox <gi...@apache.org>.
dramaticlly commented on code in PR #4650:
URL: https://github.com/apache/iceberg/pull/4650#discussion_r860026574


##########
python/README.md:
##########
@@ -35,9 +35,22 @@ pip install -e .
 Testing is done using tox. The config can be found in `tox.ini` within the python directory of the iceberg project.
 
 ```
-# simply run tox within the python dir
 tox
 ```
 
+## Solution for `InterpreterNotFound` Errors
+
+Currently, tests run against python `3.7.12`, `3.8.12`, and `3.9.10`. It's recommended to install and manage multiple interpreters using [pyenv](https://github.com/pyenv/pyenv).
+```
+pyenv install 3.7.12
+pyenv install 3.8.12
+pyenv install 3.9.10
+```
+
+Once all three versions are installed, you can set an application-specific pyenv environment by running the following in the python directory.
+```
+pyenv local 3.7.12 3.8.12 3.9.10
+```

Review Comment:
   looks great, I am thinking maybe move `tox` after pyenv setup will be a good way to validate :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] samredai commented on pull request #4650: Solution for InterpreterNotFound errors from tox

Posted by GitBox <gi...@apache.org>.
samredai commented on PR #4650:
URL: https://github.com/apache/iceberg/pull/4650#issuecomment-1111162830

   cc: @cabhishek @CircArgs @dramaticlly @rdblue 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] cabhishek commented on a diff in pull request #4650: Solution for InterpreterNotFound errors from tox

Posted by GitBox <gi...@apache.org>.
cabhishek commented on code in PR #4650:
URL: https://github.com/apache/iceberg/pull/4650#discussion_r859983691


##########
python/README.md:
##########
@@ -35,9 +35,22 @@ pip install -e .
 Testing is done using tox. The config can be found in `tox.ini` within the python directory of the iceberg project.
 
 ```
-# simply run tox within the python dir
 tox
 ```
 
+## Solution for `InterpreterNotFound` Errors
+
+Currently, tests run against python `3.7.12`, `3.8.12`, and `3.9.10`. It's recommended to install and manage multiple interpreters using [pyenv](https://github.com/pyenv/pyenv).
+```
+pyenv install 3.7.12
+pyenv install 3.8.12
+pyenv install 3.9.10

Review Comment:
   Thanks for the PR. Wondering if versioning needs to be strict up to patch level? or just 3.8.* should suffice? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] kbendick commented on a diff in pull request #4650: Solution for InterpreterNotFound errors from tox

Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4650:
URL: https://github.com/apache/iceberg/pull/4650#discussion_r860204565


##########
python/README.md:
##########
@@ -35,9 +35,22 @@ pip install -e .
 Testing is done using tox. The config can be found in `tox.ini` within the python directory of the iceberg project.
 
 ```
-# simply run tox within the python dir
 tox
 ```
 
+## Solution for `InterpreterNotFound` Errors
+
+Currently, tests run against python `3.7.12`, `3.8.12`, and `3.9.10`. It's recommended to install and manage multiple interpreters using [pyenv](https://github.com/pyenv/pyenv).
+```
+pyenv install 3.7.12
+pyenv install 3.8.12
+pyenv install 3.9.10

Review Comment:
   I do think it would be ok to do without the strict patch level.
   
   That said, since we do test against these specific patch versions, I also don't see a problem with setting it. We'll probably want to keep this up to date though.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] dramaticlly commented on pull request #4650: Solution for InterpreterNotFound errors from tox

Posted by GitBox <gi...@apache.org>.
dramaticlly commented on PR #4650:
URL: https://github.com/apache/iceberg/pull/4650#issuecomment-1111244129

   Also, not relate to this PR but this line [`pip install -e .`](https://github.com/apache/iceberg/blob/master/python/README.md?plain=1#L30)  no longer works because of we removed `setup.py` in #4285 
   
   here's my local run
   ```
   ➜  python git:(master) ✗ pip install -e .
   ERROR: File "setup.py" not found. Directory cannot be installed in editable mode: /Users/stevezhang/workspace/iceberg/python
   (A "pyproject.toml" file was found, but editable mode currently requires a setup.py based build.)
   ```
   
   I am thinking maybe we shall remove that in python `README.md` 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue merged pull request #4650: Solution for InterpreterNotFound errors from tox

Posted by GitBox <gi...@apache.org>.
rdblue merged PR #4650:
URL: https://github.com/apache/iceberg/pull/4650


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org