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: Switch to pyproject.toml #290

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ This document is formatted according to the principles of [Keep A CHANGELOG](htt
### Added
- [PHP, Java, Ruby, JavaScript] update dependency messages up to v26
- [Python] Added type annotations ([#283](https://github.com/cucumber/gherkin/pull/283))
- [Python] Switch to pyproject.toml ([#290](https://github.com/cucumber/gherkin/pull/290))

### Changed
- [.NET] Drop unsupported frameworks. Now supported target frameworks are .NET 8, .NET Standard 2.0 ([#265](https://github.com/cucumber/gherkin/pull/265))
Expand All @@ -22,6 +23,7 @@ This document is formatted according to the principles of [Keep A CHANGELOG](htt

### Removed
- [Python] Drop compatibility for python 2. Supported python versions are 3.8, 3.9, 3.10, 3.12
- [Python] Removed installation of `gherkin` script. It was used for internal acceptance tests only.
Copy link
Contributor

Choose a reason for hiding this comment

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

BAD IDEA: Removing the scripts from the package description

  • The user of this package wants to use the scripts and not some module statements that he/she does not know
  • Using gherkin/__main__.py should be kept because that is the natural choice for the main program
  • Usability for the user sucks if you do that
  • It might be OK for the repo development in Makefile but even there, it is rather suboptimal (IMHO).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, I'm even more inclined to not include the scripts/ folder in the distribution at all.

The reason is that these scripts are only used internally, as there is absolutely no API or documentation about these. Hell, I can't even figure out what the gherkin script does at all, since there is no documentation.

I really doubt that any downstream user is using these. If anything, we are just polluting their PATH with this irrelevant gherkin executable.

In the remote hypothesis that a downstream user was relying on this, we can always release a hotfix with the script included in the distribution.

Copy link
Contributor

@jsa34 jsa34 Sep 29, 2024

Choose a reason for hiding this comment

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

If the executable is needed for the purposes of the user to understand how to run main, wouldn't this be better handled with documentation?

I may have missed something, but I didn't use this executable or use it to understand how to use gherkin when I implemented it in pytest-bdd. I'm not entirely sure how useful it for including downstream from personal experience, unless I've missed a point.

Copy link
Contributor

Choose a reason for hiding this comment

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

If these scripts/executables are only for internal use, then why put them in the Python package and not put the in bin/ directory where they were before (at least one of them).

In addition, the gherkin script is provided in the current package version and before. Therefore, the script was part of the “API” of this package. By removing them you basically break this API in the next version. And you should be aware of that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If these scripts/executables are only for internal use, then why put them in the Python package and not put the in bin/ directory where they were before (at least one of them

I can put them out of the gherkin package, so that they will automatically be out of the distributed package.

In addition, the gherkin script is provided in the current package version and before. Therefore, the script was part of the “API” of this package. By removing them you basically break this API in the next version. And you should be aware of that.

yes I know, but every release is a major release it seems, so semver semantics allow this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can put them out of the gherkin package, so that they will automatically be out of the distributed package.

Done in b94911b


## [29.0.0] - 2024-08-12
### Added
Expand Down
28 changes: 0 additions & 28 deletions python/MANIFEST

This file was deleted.

12 changes: 6 additions & 6 deletions python/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ GHERKIN_PARSER = gherkin/parser.py
GHERKIN_RAZOR = gherkin-python.razor
SOURCE_FILES = $(shell find . -name "*.py" | grep -v $(GHERKIN_PARSER))

GHERKIN = bin/gherkin
GHERKIN_GENERATE_TOKENS = bin/gherkin-generate-tokens
GHERKIN_GENERATE_EVENTS = python -m scripts.generate_events
GHERKIN_GENERATE_TOKENS = python -m scripts.generate_tokens

GOOD_FEATURE_FILES = $(shell find ../testdata/good -name "*.feature")
BAD_FEATURE_FILES = $(shell find ../testdata/bad -name "*.feature")
Expand Down Expand Up @@ -56,20 +56,20 @@ acceptance/testdata/%.tokens: ../testdata/% ../testdata/%.tokens

acceptance/testdata/%.ast.ndjson: ../testdata/% ../testdata/%.ast.ndjson
mkdir -p $(@D)
$(GHERKIN) --no-source --no-pickles $< | jq --sort-keys --compact-output "." > $@
$(GHERKIN_GENERATE_EVENTS) --no-source --no-pickles $< | jq --sort-keys --compact-output "." > $@
diff --unified <(jq "." $<.ast.ndjson) <(jq "." $@)

acceptance/testdata/%.pickles.ndjson: ../testdata/% ../testdata/%.pickles.ndjson
mkdir -p $(@D)
$(GHERKIN) --no-source --no-ast $< | jq --sort-keys --compact-output "." > $@
$(GHERKIN_GENERATE_EVENTS) --no-source --no-ast $< | jq --sort-keys --compact-output "." > $@
diff --unified <(jq "." $<.pickles.ndjson) <(jq "." $@)

acceptance/testdata/%.source.ndjson: ../testdata/% ../testdata/%.source.ndjson
mkdir -p $(@D)
$(GHERKIN) --no-ast --no-pickles $< | jq --sort-keys --compact-output "." > $@
$(GHERKIN_GENERATE_EVENTS) --no-ast --no-pickles $< | jq --sort-keys --compact-output "." > $@
diff --unified <(jq "." $<.source.ndjson) <(jq "." $@)

acceptance/testdata/%.errors.ndjson: ../testdata/% ../testdata/%.errors.ndjson
mkdir -p $(@D)
$(GHERKIN) --no-source $< | jq --sort-keys --compact-output "." > $@
$(GHERKIN_GENERATE_EVENTS) --no-source $< | jq --sort-keys --compact-output "." > $@
diff --unified <(jq "." $<.errors.ndjson) <(jq "." $@)
15 changes: 0 additions & 15 deletions python/bin/gherkin

This file was deleted.

15 changes: 0 additions & 15 deletions python/bin/gherkin-generate-tokens

This file was deleted.

13 changes: 0 additions & 13 deletions python/bin/gherkin_generate_tokens.py

This file was deleted.

46 changes: 0 additions & 46 deletions python/gherkin/__main__.py

This file was deleted.

44 changes: 44 additions & 0 deletions python/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,47 @@
[build-system]
requires = ["setuptools>=61.0", "wheel"]
build-backend = "setuptools.build_meta"

[project]
name = "gherkin-official"
version = "29.0.0"
youtux marked this conversation as resolved.
Show resolved Hide resolved
description = "Gherkin parser (official, by Cucumber team)"
readme = "README.md"
requires-python = ">=3.8"
authors = [
{ name = "Cucumber Ltd and Björn Rasmusson", email = "[email protected]" }
]

dependencies = [
"typing_extensions>=4",
]
keywords = ["gherkin", "cucumber", "bdd"]
classifiers = [
"License :: OSI Approved :: MIT License",
"Programming Language :: Python",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12"
]

[project.urls]
Homepage = "https://github.com/cucumber/gherkin"
Repository = "https://github.com/cucumber/gherkin.git"
Download = "https://pypi.python.org/pypi/gherkin-official"
Issues = "https://github.com/cucumber/gherkin/issues"
Changelog = "https://github.com/cucumber/gherkin/releases"

[tool.setuptools.packages.find]
include = ["gherkin", "gherkin.*"]

[tool.mypy]
disallow_untyped_defs = true
packages = ["gherkin"]


[tool.black]
# Using `(python/)?` to match both `gherkin/parser.py` and `python/gherkin/parser.py`.
# This is needed to be able to run `black` as a pre-commit hook, and also to run it manually.
Expand Down
54 changes: 54 additions & 0 deletions python/scripts/generate_events.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
from optparse import OptionParser
import json

from gherkin.stream.gherkin_events import GherkinEvents
from gherkin.stream.source_events import SourceEvents


def create_arg_parser() -> OptionParser:
parser = OptionParser()
parser.add_option(
"--no-source",
action="store_false",
dest="print_source",
default=True,
help="don't print source events",
)
parser.add_option(
"--no-ast",
action="store_false",
dest="print_ast",
default=True,
help="don't print ast events",
)
parser.add_option(
"--no-pickles",
action="store_false",
dest="print_pickles",
default=True,
help="don't print pickle events",
)
return parser


def main() -> None:
arg_parser = create_arg_parser()

options, args = arg_parser.parse_args()

source_events = SourceEvents(args)
gherkin_events = GherkinEvents(
GherkinEvents.Options(
print_source=options.print_source,
print_ast=options.print_ast,
print_pickles=options.print_pickles,
)
)

for source_event in source_events.enum():
for event in gherkin_events.enum(source_event):
print(json.dumps(event))


if __name__ == "__main__":
main()
17 changes: 17 additions & 0 deletions python/scripts/generate_tokens.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import sys

from gherkin.token_scanner import TokenScanner
from gherkin.token_formatter_builder import TokenFormatterBuilder
from gherkin.parser import Parser


def main() -> None:
files = sys.argv[1:]
parser = Parser(TokenFormatterBuilder())
for file in files:
scanner = TokenScanner(file)
print(parser.parse(scanner))


if __name__ == "__main__":
main()
6 changes: 0 additions & 6 deletions python/setup.cfg

This file was deleted.

31 changes: 0 additions & 31 deletions python/setup.py

This file was deleted.