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 executable cleanups #33

Merged
merged 4 commits into from Apr 20, 2023
Merged

Python executable cleanups #33

merged 4 commits into from Apr 20, 2023

Conversation

Chroxvi
Copy link
Contributor

@Chroxvi Chroxvi commented Apr 19, 2023

This is a follow-up to #30 to fully implement PEP394 in cotainr.

  • Use python3 executable instead of python.
  • Use sys.executable when applicable
  • Update docs to reflect that we implement PEP394
  • Added an error early check to produce a meaningful error message when trying to run cotainr using a too old version of Python, i.e. instead of
    Traceback (most recent call last):
      File "./bin/cotainr", line 12, in <module>
        from cotainr.cli import main
      File "<fstring>", line 1
        (sub_cmd=)
                ^
    SyntaxError: invalid syntax
    you now get something like
    Cotainr requires Python>=3.8.0
    You are running Python==3.7.12 | packaged by conda-forge | (default, Oct 26 2021, 06:08:53) 
    [GCC 9.4.0]
    from '/miniconda3/envs/py37_cotainr_test/bin/python3'
    ABORTING!

@Chroxvi Chroxvi requested review from rloewe and eskech April 19, 2023 07:32
Copy link

@ankit-gautam23 ankit-gautam23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@eskech eskech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add documentation change also to state that we now only support python3

@@ -7,6 +7,8 @@

"""

import sys

__version__ = "2023.02.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we waiting for the output change to create a new release?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we have #33, #34, and #36 staged for release when approved and merged. None of those seem critical to release ASAP, so I suggest we at least include any needed fix for #35 and possibly also a reworked version of #32 before making a new release.

@Chroxvi
Copy link
Contributor Author

Chroxvi commented Apr 20, 2023

Add documentation change also to state that we now only support python3

We have always only supported python>=3.8 as stated on https://cotainr.readthedocs.io/en/latest/user_guide/index.html#cotainr-dependencies. The only change is that we now explicitly call python3 instead of relying on python to be a symlink to python3 as python may or may not exist and may or may not point to python2 instead of python3.

@Chroxvi Chroxvi merged commit 0216c7f into main Apr 20, 2023
11 checks passed
@Chroxvi Chroxvi deleted the python_executable_cleanups branch April 20, 2023 12:00
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.

None yet

3 participants