Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

add __version__ and version_info #44

Closed
wants to merge 3 commits into from
Closed

add __version__ and version_info #44

wants to merge 3 commits into from

Conversation

AvdN
Copy link

@AvdN AvdN commented Jun 29, 2017

This change allows programs and other packages to do:

  import typed_ast
  if typed_ast.version_info < (1, 1):
      print(typed_ast.__version__)

currently the only way to get that version number is by doing

  import pkg_resources
  pkg_resources.require('typed-ast')[0].version)

(pr on mypy, rejected at least partly, because of the difficulty of extracting the typed_ast version number)

This change allows programs and other packages to do:

  import typed_ast
  if typed_ast.version_info < (1, 1):
      print(typed_ast.__version__)

currently the only way to get that version number is by doing

  import pkg_resources
  pkg_resources.require('typed-ast')[0].version)
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

Copy link
Author

@AvdN AvdN left a comment

Choose a reason for hiding this comment

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

no changes, just updated Details on b.p.o, CLA had already been signed. Please rerun the bot

@ddfisher
Copy link
Collaborator

Thanks for this PR, and sorry about the long wait for review! We'd prefer to avoid doing manual parsing of the version -- is there some reason you're avoiding importing the file directly?

@AvdN
Copy link
Author

AvdN commented Jul 19, 2017

It is just my experience that it leads to problems if you import directly. Maybe not in your case now, but if __init__.py imports another package "ABC", and you install typed_ast a new virtualenv, where "ABC" is not installed yet, then "ABC" needs to be installed by hand before setup.py can run, because setup.py cannot run because it is dependent on that package via __init__.py. And if the environment in which you create your package has "ABC", you'll never noticed that until you install the package in a clean environment (e.g. if you run tox for testing)

Now you might be sure such a dependency will never happen. and that you will notice, but why run the risk? It is not as if the performance of setup.py is influenced that much now by doing the parsing.

@gvanrossum
Copy link
Member

The worry is imagined, but the complexity introduced to avoid this potential future problem is real. Let's do the simple thing (import typed_ast.version).

@ambv
Copy link

ambv commented Jul 19, 2017

I settled on doing the following in all my packages:

_version_re = re.compile(r'__version__\s+=\s+(?P<version>.*)')

with open(os.path.join(current_dir, 'retype.py'), 'r', encoding='utf8') as f:
    version = _version_re.search(f.read()).group('version')
    version = str(ast.literal_eval(version))

This is what Flask, Click and a few other very popular packages are also doing.

@ethanhs
Copy link
Contributor

ethanhs commented Jul 19, 2017

Yes, but that is due to concerns about importing packages that may not exist at runtime. I cant see this happening for typed_ast so I agree with Guido, I see no reason for unnecessary complexity.

@gvanrossum
Copy link
Member

gvanrossum commented Jul 19, 2017 via email

@ddfisher
Copy link
Collaborator

Agreed -- let's go with @ambv's suggestion. @AvdN: want to update this diff?

@AvdN
Copy link
Author

AvdN commented Jul 19, 2017

@ddfisher I am not sure if I understand where this code goes and what it helps if the version is in a file retype.py. In the end you want to be able to do from typed_ast import __version__ and for that __version__ has to be in __init__.py), I also can't imagine you want to clutter your sources with yet another file just to contain the version number, YMMV.

As my PR already has 2 revisions more than should have been necessary, I suggest you just directly put that code in at some point instead of me proposing another revision, that might not be acceptable because I don't understand where the retype.py comes from. Most important is for typed_ast to have an easy accessible version number, not that it is my PR that gets it there (but thanks for asking first)

Don't forget to import re, ast and os in setup.py (assuming you put the code ambv proposed is put in there).

@ambv
Copy link

ambv commented Jul 19, 2017

retype is one of my packages, I just copy-pasta'd the snippet from its setup.py to show how it works. retype.py is irrelevant to this project. You would use typed_ast/__init__.py.

@ddfisher
Copy link
Collaborator

I took a swing at it in #48. (Closing in favor of that.)

@ddfisher ddfisher closed this Jul 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants