-
Notifications
You must be signed in to change notification settings - Fork 1.2k
add PEP-396 version attribute to sagemaker module #435
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #435 +/- ##
=========================================
- Coverage 93.75% 93.6% -0.15%
=========================================
Files 55 55
Lines 4032 4033 +1
=========================================
- Hits 3780 3775 -5
- Misses 252 258 +6
Continue to review full report at Codecov.
|
863d1ff
to
a8fc413
Compare
|
||
from setuptools import setup, find_packages | ||
|
||
|
||
def get_version(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should follow pep-0396 guidance https://www.python.org/dev/peps/pep-0396/#classic-distutils :
In classic distutils, the simplest way to add the version string to the setup() function in setup.py is to do something like this:
from elle import __version__
setup(name='elle', version=__version__)
In the PEP author's experience however, this can fail in some cases, such as when the module uses automatic Python 3 conversion via the 2to3 program (because setup.py is executed by Python 3 before the elle module has been converted).
In that case, it's not much more difficult to write a little code to parse the version from the file rather than importing it. Without providing too much detail, it's likely that modules such as distutils2 will provide a way to parse version strings from files. E.g.:
from distutils2 import get_version
setup(name='elle', version=get_version('elle.py'))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, from offline conversation that seems to be the new way (especially since Distutils2 development is stopped):
https://packaging.python.org/guides/single-sourcing-package-version/
Fix typo in advanced_functionality/r_bring_your_own/README.md
Issue #, if available:
#434
Description of changes:
add PEP-396 version attribute to sagemaker module
Merge Checklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.