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

Add messages implementation for python #165

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
23 changes: 23 additions & 0 deletions .github/workflows/release-pypi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Release Python

on:
push:
branches: [release/*]

jobs:
release:
name: Release
runs-on: ubuntu-latest
environment: Release
permissions:
id-token: write
defaults:
run:
working-directory: python
steps:
- name: Checkout code
uses: actions/checkout@v4

- uses: cucumber/[email protected]
with:
working-directory: "python"
63 changes: 63 additions & 0 deletions .github/workflows/test-python.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
---
name: test-python

on:
push:
branches:
- main
- renovate/**
pull_request:
branches:
- main
workflow_dispatch:

jobs:
build:

runs-on: ${{ matrix.os }}
strategy:
matrix:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
matrix:
matrix:
include:
# Test latest python on windows / macos
- { os: 'windows-latest', python-version: '3.13' }
- { os: 'macos-latest', python-version: '3.13' }
os: ['ubuntu-latest]
python-version: ['3.9',, '3.10', '3.11', '3.12', '3.13', 'pypy3.9', 'pypy3.10']

include:
- os: ubuntu-latest
python-version: "3.9"
- os: ubuntu-latest
python-version: "3.10"
- os: ubuntu-latest
python-version: "3.11"
- os: ubuntu-latest
python-version: "3.12"
- os: ubuntu-latest
python-version: "3.13"
- os: ubuntu-latest
python-version: "pypy3.9"
- os: ubuntu-latest
python-version: "pypy3.10"
- os: windows-latest
python-version: "3.13"
- os: macos-latest
python-version: "3.13"

steps:
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Generate code
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be needed as this should be part of the regular code generation and checkin process

Copy link
Author

Choose a reason for hiding this comment

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

It's needed for test execution later: We have to know if the model is fine for serialization/deserialization

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I think you're misunderstanding. The code "itself" is checked in (See this PR below for the code _messages.py I think is the single file with all messages.

As part of your regular development flow you can develop new models and/or new facilities for enum's e.t.c. - Then once you run make generate it will generate all your new code. This is the same flow as every other language

This code (Running make generate), would therefore "re"generate the code on each and every PR which isn't something we would want to do, as it would create a rather large overhead. So you should just run make generate whenever you want to update your messages

Copy link
Author

@elchupanebrej elchupanebrej Jan 21, 2025

Choose a reason for hiding this comment

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

Aha! Sorry, I've missed that context, thank you!

working-directory: ./python
run: |
make clean
make generate
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -U setuptools
pip install tox tox-gh-actions codecov
- name: Test with tox
working-directory: ./python
run: |
tox
- name: Gather codecov report
working-directory: ./python
run: |
codecov
38 changes: 38 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# See https://pre-commit.com for more information
luke-hill marked this conversation as resolved.
Show resolved Hide resolved
# See https://pre-commit.com/hooks.html for more hooks
---
files: ^python/
exclude: .*python/src/cucumber_messages/_messages\.py
repos:
- repo: https://github.com/psf/black
rev: 24.10.0
hooks:
- id: black
args:
- "python/src"
- "python/tests"
- repo: https://github.com/pycqa/isort
rev: 5.13.2
hooks:
- id: isort
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-added-large-files
- repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks
rev: v2.14.0
hooks:
- id: pretty-format-toml
args: [--autofix]
- repo: https://github.com/asottile/pyupgrade
rev: v3.19.1
hooks:
- id: pyupgrade
args: ["--py39-plus"]
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.13.0
hooks:
- id: mypy
additional_dependencies: [types-setuptools, types-certifi]
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased]
### Added
- [cpp] use VERSION file to version ABI and shared libraries (#268](https://github.com/cucumber/messages/pull/268)
- [python] Added Python implementation ([#165](https://github.com/cucumber/messages/pull/165))

## [27.0.2] - 2024-11-15
### Fixed
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ schemas = \
./jsonschema/UndefinedParameterType.json \
./jsonschema/Envelope.json

languages = cpp go java javascript perl php ruby dotnet
languages = cpp dotnet go java javascript perl php python ruby

.DEFAULT_GOAL = help

Expand Down
1 change: 1 addition & 0 deletions codegen/codegen.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
require_relative 'generators/markdown'
require_relative 'generators/perl'
require_relative 'generators/php'
require_relative 'generators/python'
require_relative 'generators/ruby'
require_relative 'generators/typescript'

Expand Down
159 changes: 159 additions & 0 deletions codegen/generators/python.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
# frozen_string_literal: true

module Generator
class Python < Base
def format_enum_value(value)
value.downcase.gsub(/[.\/+\s-]/, '_')
end

def get_sorted_properties(definition)
required_fields = definition['required'] || []
definition['properties'].sort_by do |name, *|
[required_fields.include?(name) ? 0 : 1, name]
end
end

def format_property(parent_type_name, property_name, property, required_fields)
snake_name = property_name.gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2')
.gsub(/([a-z\d])([A-Z])/, '\1_\2')
.downcase

property_type = get_property_type(parent_type_name, property_name, property)
is_required = required_fields.include?(property_name)

if is_required
"#{snake_name}: #{property_type}"
else
"#{snake_name}: Optional[#{property_type}] = None"
end
end

def get_property_type(parent_type_name, property_name, property)
type = type_for(parent_type_name, property_name, property)

if type.start_with?('list[')
list_type = type.match(/list\[(.*?)\]/)
luke-hill marked this conversation as resolved.
Show resolved Hide resolved
inner_type = list_type[1]
if inner_type =~ /^[A-Z]/
luke-hill marked this conversation as resolved.
Show resolved Hide resolved
"list[\"#{class_name(inner_type)}\"]"
else
"list[#{inner_type}]"
end
elsif type =~ /^[A-Z]/
luke-hill marked this conversation as resolved.
Show resolved Hide resolved
"\"#{class_name(type)}\""
else
type
end
end

def array_type_for(type_name)
inner_type = if language_translations_for_data_types.values.include?(type_name)
type_name # Keep primitive types as is
else
class_name(type_name) # CamelCase for complex types
end
"list[#{inner_type}]"
end

def format_description(raw_description, indent_string: ' ')
return '""" """' if raw_description.nil?

lines = raw_description.split("\n").map { |line|
if line.strip.empty?
""
else
"#{indent_string}#{line.rstrip}"
end
}

%("""\n#{lines.join("\n")}\n#{indent_string}""")
end

def language_translations_for_data_types
{
'integer' => 'int',
'string' => 'str',
'boolean' => 'bool',
'array' => 'list'
}
end

private

def default_value(parent_type_name, property_name, property)
if property['type'] == 'string'
default_value_for_string(parent_type_name, property_name, property)
elsif property['type'] == 'integer'
'0'
elsif property['type'] == 'boolean'
'False'
elsif property['type'] == 'array'
'[]'
elsif property['$ref']
"#{class_name(type_for(parent_type_name, nil, property))}()"
else
'None'
end
end

def default_value_for_string(parent_type_name, property_name, property)
if property['enum']
enum_type_name = type_for(parent_type_name, property_name, property)
"#{class_name(enum_type_name)}.#{enum_constant(property['enum'][0])}"
else
'""'
end
end

def type_for(parent_type_name, property_name, property)
if property['$ref']
property_type_from_ref(property['$ref'])
elsif property['type']
property_type_from_type(parent_type_name, property_name, property, type: property['type'])
else
raise "Property #{property_name} did not define 'type' or '$ref'"
end
end

def property_type_from_type(parent_type_name, property_name, property, type:)
if type == 'array'
array_type_for(type_for(parent_type_name, nil, property['items']))
elsif property['enum']
enum_name(parent_type_name, property_name, property['enum'])
else
language_translations_for_data_types.fetch(type)
end
end

def enum_constant(value)
value.gsub(/[.\/+]/, '_').downcase
end

def enum_name(parent_type_name, property_name, enum)
"#{class_name(parent_type_name)}#{capitalize(property_name)}".tap do |name|
@enum_set.add({ name: name, values: enum })
end
end

def property_type_from_ref(ref)
class_name(ref)
end

def class_name(ref)
return ref if language_translations_for_data_types.values.include?(ref)

# Remove .json extension if present
name = ref.sub(/\.json$/, '')
# Get the basename without path
name = File.basename(name)
# Convert each word to proper case, handling camelCase and snake_case
parts = name.gsub(/[._-]/, '_').split('_').map do |part|
# Split by any existing camelCase
subparts = part.scan(/[A-Z][a-z]*|[a-z]+/)
subparts.map(&:capitalize).join
end
# Join all parts to create final CamelCase name
parts.join
end
end
end
41 changes: 41 additions & 0 deletions codegen/templates/python.py.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# This code was generated using the code generator from cucumber-messages.
Copy link
Contributor

Choose a reason for hiding this comment

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

Whilst it's not mandatory usually we split the two templates up into an enum template and a non-enum (Standard class), template.

Usually this is because the enums generate just some simple readers for each constant whereas the class usually has some primitive behaviour (like serialization / deserialization).

An example of the complexity difference in ruby would be these two

ENUM: Just has constant definitions https://github.com/cucumber/messages/blob/main/ruby/lib/cucumber/messages/attachment_content_encoding.rb

Class: Has some basic readers/serialization logic (Note the attr_readers and the .from_h method https://github.com/cucumber/messages/blob/main/ruby/lib/cucumber/messages/attachment.rb

Copy link
Author

@elchupanebrej elchupanebrej Dec 19, 2024

Choose a reason for hiding this comment

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

The template is really simple, and all serialization/deserialization is done externally. Actually https://pypi.org/project/marshmallow-dataclass/ could be used instead of proposed serialization/deserialization approach

Copy link
Contributor

Choose a reason for hiding this comment

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

Whilst the template is simple for all of the generators, we still partition them. Again it would be best if we keep things structured in the same way imo because then we can clearly see what moving parts support which portion of the codebase

# Manual changes will be lost if the code is regenerated.
# Generator: cucumber-messages-python

from __future__ import annotations
from dataclasses import dataclass
from enum import Enum
from typing import Optional


<%- @enums.each do |enum| -%>
class <%= enum[:name] %>(Enum):
<%- enum[:values].each do |value| -%>
<%= format_enum_value(value) %> = "<%= value %>"
<%- end -%>


<%- end -%>
<%- @schemas.each_with_index do |schema_pair, index| -%>
<%- key, definition = schema_pair -%>
@dataclass
class <%= class_name(key) %>:
<%- if definition['description'] -%>
<%= format_description(definition['description']) %>
<%- end -%>
<%- if definition['properties'].any? -%>
<%- required_fields = definition['required'] || [] -%>
<%- get_sorted_properties(definition).each do |property_name, property| -%>
<%- if property['description'] -%>
<%= format_description(property['description']) %>
<%- end -%>
<%= format_property(key, property_name, property, required_fields) %>
<%- end -%>
<%- else -%>
pass
<%- end -%>
<%- if index < @schemas.length - 1 -%>


<%- end -%>
<%- end -%>
Empty file modified cpp/cmake/cmate
100755 β†’ 100644
Empty file.
Loading
Loading