-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
Pyproject support #34
Changes from 7 commits
b2e74b5
7d36495
45540d7
fd5c470
51c045f
f54b957
d8bc619
a5a6565
b15d091
08a92d5
16f0c7d
76c40f2
483d27b
0aca989
ac1b95d
dad4492
8a515de
7ea9050
12c5265
ed48eda
d748483
583c366
b10da17
8349504
c7e1623
82c67ea
63e735a
7ed0b9a
605fe9a
c166aed
f2c7d21
2a0d7d4
1214b38
572ca6e
7e471e4
4539a49
f1cb32e
de13d7a
04c8988
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
[tool.poetry] | ||
name = "check-python-versions" | ||
version = "1.0.0" | ||
homepage = "https://github.com/mgedmin/check-python-versions" | ||
description = "Compare supported Python versions in setup.py vs tox.ini et al." | ||
authors = ["Marius Gedminas <[email protected]>"] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,8 @@ | |
check-python-versions supports both. | ||
""" | ||
|
||
import pytomlpp | ||
from tomlkit import dumps | ||
from tomlkit import parse, load | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. flake8 tells me tomlkit.parse is imported but unused. |
||
|
||
from typing import ( | ||
List, | ||
|
@@ -38,11 +39,29 @@ | |
PYPROJECT_TOML = 'pyproject.toml' | ||
|
||
|
||
def _load_toml(filename: FileOrFilename): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice if there was a type annotation for the return type. I suppose it's hard to be more specific than a generic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems this row is outdated, also the file was renamed to "pyproject.py" because it handles all the tools that use pyproject.toml |
||
table = {} | ||
# tomlkit has two different API to load from file name or file object | ||
if isinstance(filename, str): | ||
with open_file(filename) as fp: | ||
table = load(fp) | ||
if isinstance(filename, TextIO): | ||
table = load(filename) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a |
||
return table | ||
|
||
|
||
def get_toml_content( | ||
filename: FileOrFilename = PYPROJECT_TOML | ||
) -> FileLines: | ||
"""Utility method to see if TOML library keeps style and comments.""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's only used by the test suite, it should be moved into test_pyproject.py. |
||
table = _load_toml(filename) | ||
return dumps(table).split('\n') | ||
|
||
|
||
def _get_pyproject_toml_classifiers( | ||
filename: FileOrFilename = PYPROJECT_TOML | ||
filename: FileOrFilename = PYPROJECT_TOML | ||
) -> List[str]: | ||
with open_file(filename) as f: | ||
table = pytomlpp.load(f) | ||
table = _load_toml(filename) | ||
|
||
if 'tool' not in table: | ||
return [] | ||
|
@@ -55,10 +74,9 @@ def _get_pyproject_toml_classifiers( | |
|
||
|
||
def _get_pyproject_toml_python_requires( | ||
filename: FileOrFilename = PYPROJECT_TOML | ||
filename: FileOrFilename = PYPROJECT_TOML | ||
) -> List[str]: | ||
with open_file(filename) as f: | ||
table = pytomlpp.load(f) | ||
table = _load_toml(filename) | ||
|
||
if 'tool' not in table: | ||
return [] | ||
|
@@ -83,6 +101,10 @@ def get_supported_python_versions( | |
# versions in classifiers. | ||
return [] | ||
|
||
if not isinstance(classifiers, (list, tuple)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does TOML support tuples? Somehow I thought it only had lists ("arrays") and dicts ("tables") as data structures. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you're right, tuples are not supported |
||
warn('The value passed to classifiers is not a list') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'passed to' phrasing makes sense when talking about values passed to Python function calls, which is what happens in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't used TOML much. Are there alternative syntaxes? E.g. could I write something like [tools]
poetry = { classifiers: [...] } or [tools.poetry.classifiers]
... instead? If so maybe the message should say "The value of tools.poetry.classifiers is not an array", or whatever is the common syntax for identifying a piece of a nested data structure in TOML-land. |
||
return [] | ||
|
||
return get_versions_from_classifiers(classifiers) | ||
|
||
|
||
|
@@ -94,7 +116,7 @@ def get_python_requires( | |
if python_requires is None: | ||
return None | ||
if not isinstance(python_requires, str): | ||
warn('The value passed to python is not a string') | ||
warn('The value passed to python dependency is not a string') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise. |
||
return None | ||
return parse_python_requires(python_requires) | ||
|
||
|
@@ -111,7 +133,7 @@ def update_supported_python_versions( | |
if classifiers is None: | ||
return None | ||
if not isinstance(classifiers, (list, tuple)): | ||
warn('The value passed to setup(classifiers=...) is not a list') | ||
warn('The value passed to classifiers is not a list') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise. |
||
return None | ||
new_classifiers = update_classifiers(classifiers, new_versions) | ||
return _update_pyproject_toml_classifiers(filename, new_classifiers) | ||
|
@@ -126,7 +148,7 @@ def update_python_requires( | |
Does not touch the file but returns a list of lines with new file contents. | ||
""" | ||
python_requires = _get_pyproject_toml_python_requires(filename) | ||
if python_requires is None: | ||
if python_requires is None or python_requires == []: | ||
return None | ||
comma = ', ' | ||
if ',' in python_requires and ', ' not in python_requires: | ||
|
@@ -144,11 +166,10 @@ def update_python_requires( | |
|
||
|
||
def _update_pyproject_toml_classifiers( | ||
filename: FileOrFilename, | ||
new_value: Union[str, List[str]] | ||
filename: FileOrFilename, | ||
new_value: Union[str, List[str]], | ||
) -> Optional[FileLines]: | ||
with open_file(filename) as f: | ||
table = pytomlpp.load(f) | ||
table = _load_toml(filename) | ||
|
||
if 'tool' not in table: | ||
return [] | ||
|
@@ -157,15 +178,14 @@ def _update_pyproject_toml_classifiers( | |
|
||
table['tool']['poetry']['classifiers'] = new_value | ||
|
||
return pytomlpp.dumps(table).split('\n') | ||
return dumps(table).split('\n') | ||
|
||
|
||
def _update_pyproject_toml_python_requires( | ||
filename: FileOrFilename, | ||
new_value: Union[str, List[str]] | ||
new_value: Union[str, List[str]], | ||
) -> Optional[FileLines]: | ||
with open_file(filename) as f: | ||
table = pytomlpp.load(f) | ||
table = _load_toml(filename) | ||
|
||
if 'tool' not in table: | ||
return [] | ||
|
@@ -175,7 +195,7 @@ def _update_pyproject_toml_python_requires( | |
return [] | ||
|
||
table['tool']['poetry']['dependencies']['python'] = new_value | ||
return pytomlpp.dumps(table).split('\n') | ||
return dumps(table).split('\n') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: investigate what tomlkit.dumps() returns and whether this should be using .splitlines() instead. Note to self: make sure the unit tests verify that modifying a .toml file doesn't add or remove any trailing newlines at the end of the FILE. |
||
|
||
|
||
PoetryPyProject = Source( | ||
|
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.
It's not the version that's a problem, it's that this project (check-python-versions) doesn't use Poetry and therefore it shouldn't have Poetry configuration in its top-level pyproject.toml.
Please move this file into
tests/sources/data/
or a similarly named subdirectory if it's needed by the test suite.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.
it's clear that this project doesn't use poetry to build it.
I've made the file to do a live check with check-python-versions.py , so it could be related to test_cli.py , I've to look at it.