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.

Sign up for GitHub

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

Jump to bottom

THRIFT-5139/THRIFT-4181: Python3 type hints #2929

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+136 −10
Open

THRIFT-5139/THRIFT-4181: Python3 type hints #2929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04d5351
Python3 type hints
Feb 16, 2022
b0e3aae
Update tests for type_hints
arkuhn Feb 24, 2024
b8d0da3
Review comments
arkuhn May 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
128 changes: 120 additions & 8 deletions compiler/cpp/src/thrift/generate/t_py_generator.cc
View file
Open in desktop
Expand Up @@ -63,6 +63,7 @@ class t_py_generator : public t_generator {
gen_twisted_ = false;
gen_dynamic_ = false;
gen_enum_ = false;
gen_type_hints_ = false;
coding_ = "";
gen_dynbaseclass_ = "";
gen_dynbaseclass_exc_ = "";
Expand Down Expand Up @@ -125,6 +126,11 @@ class t_py_generator : public t_generator {
gen_tornado_ = true;
} else if( iter->first.compare("coding") == 0) {
coding_ = iter->second;
} else if (iter->first.compare("type_hints") == 0) {
if (!gen_enum_) {
throw "the type_hints py option requires the enum py option";
}
gen_type_hints_ = true;
} else {
throw "unknown option py:" + iter->first;
}
Expand Down Expand Up @@ -252,12 +258,16 @@ class t_py_generator : public t_generator {
std::string declare_argument(t_field* tfield);
std::string render_field_default_value(t_field* tfield);
std::string type_name(t_type* ttype);
std::string function_signature(t_function* tfunction, bool interface = false);
std::string function_signature(t_function* tfunction, bool interface = false, bool send_part = false);
std::string argument_list(t_struct* tstruct,
std::vector<std::string>* pre = nullptr,
std::vector<std::string>* post = nullptr);
std::string type_to_enum(t_type* ttype);
std::string type_to_spec_args(t_type* ttype);
std::string type_to_py_type(t_type* type);
std::string member_hint(t_type* type, t_field::e_req req);
std::string arg_hint(t_type* type);
std::string func_hint(t_type* type);

static bool is_valid_namespace(const std::string& sub_namespace) {
return sub_namespace == "twisted";
Expand Down Expand Up @@ -314,6 +324,11 @@ class t_py_generator : public t_generator {

bool gen_slots_;

/**
* True if we should generate classes type hints and type checks in write methods.
*/
bool gen_type_hints_;

std::string copy_options_;

/**
Expand Down Expand Up @@ -460,12 +475,18 @@ string t_py_generator::py_autogen_comment() {
*/
string t_py_generator::py_imports() {
ostringstream ss;
if (gen_type_hints_) {
ss << "from __future__ import annotations" << '\n' << "import typing" << '\n';
}

ss << "from thrift.Thrift import TType, TMessageType, TFrozenDict, TException, "
"TApplicationException"
<< '\n'
<< "from thrift.protocol.TProtocol import TProtocolException"
<< '\n'
<< "from thrift.TRecursive import fix_spec"
<< '\n'
<< "from uuid import UUID"
<< '\n';
if (gen_enum_) {
ss << "from enum import IntEnum" << '\n';
Expand Down Expand Up @@ -597,6 +618,9 @@ string t_py_generator::render_const_value(t_type* type, t_const_value* value) {
out << emit_double_as_string(value->get_double());
}
break;
case t_base_type::TYPE_UUID:
out << "UUID(\"" << get_escaped_string(value) << "\")";
break;
default:
throw "compiler error: no const of base type " + t_base_type::t_base_name(tbase);
}
Expand Down Expand Up @@ -882,7 +906,9 @@ void t_py_generator::generate_py_struct_definition(ostream& out,
<< "'] = " << (*m_iter)->get_name() << '\n';
}
} else {
indent(out) << "self." << (*m_iter)->get_name() << " = " << (*m_iter)->get_name() << '\n';
indent(out) << "self." << (*m_iter)->get_name()
<< member_hint((*m_iter)->get_type(), (*m_iter)->get_req()) << " = "
<< (*m_iter)->get_name() << '\n';
}
}

Expand Down Expand Up @@ -1153,7 +1179,7 @@ void t_py_generator::generate_py_struct_writer(ostream& out, t_struct* tstruct)

indent(out) << "def write(self, oprot):" << '\n';
indent_up();

indent(out) << "self.validate()" << '\n';
indent(out) << "if oprot._fast_encode is not None and self.thrift_spec is not None:" << '\n';
indent_up();

Expand Down Expand Up @@ -1560,7 +1586,7 @@ void t_py_generator::generate_service_client(t_service* tservice) {
}

f_service_ << '\n';
indent(f_service_) << "def send_" << function_signature(*f_iter, false) << ":" << '\n';
indent(f_service_) << "def send_" << function_signature(*f_iter, false, true) << ":" << '\n';
indent_up();

std::string argsname = (*f_iter)->get_name() + "_args";
Expand Down Expand Up @@ -2337,6 +2363,9 @@ void t_py_generator::generate_deserialize_field(ostream& out,
case t_base_type::TYPE_DOUBLE:
out << "readDouble()";
break;
case t_base_type::TYPE_UUID:
out << "readUuid()";
break;
default:
throw "compiler error: no Python name for base type " + t_base_type::t_base_name(tbase);
}
Expand Down Expand Up @@ -2529,6 +2558,9 @@ void t_py_generator::generate_serialize_field(ostream& out, t_field* tfield, str
case t_base_type::TYPE_DOUBLE:
out << "writeDouble(" << name << ")";
break;
case t_base_type::TYPE_UUID:
out << "writeUuid(" << name << ")";
break;
default:
throw "compiler error: no Python name for base type " + t_base_type::t_base_name(tbase);
}
Expand Down Expand Up @@ -2702,7 +2734,10 @@ void t_py_generator::generate_python_docstring(ostream& out, t_doc* tdoc) {
*/
string t_py_generator::declare_argument(t_field* tfield) {
std::ostringstream result;
result << tfield->get_name() << "=";
t_field::e_req req = tfield->get_req();
result << tfield->get_name() << member_hint(tfield->get_type(), req);

result << " = ";
if (tfield->get_value() != nullptr) {
result << render_field_default_value(tfield);
} else {
Expand Down Expand Up @@ -2731,7 +2766,7 @@ string t_py_generator::render_field_default_value(t_field* tfield) {
* @param tfunction Function definition
* @return String of rendered function definition
*/
string t_py_generator::function_signature(t_function* tfunction, bool interface) {
string t_py_generator::function_signature(t_function* tfunction, bool interface, bool send_part) {
vector<string> pre;
vector<string> post;
string signature = tfunction->get_name() + "(";
Expand All @@ -2741,6 +2776,10 @@ string t_py_generator::function_signature(t_function* tfunction, bool interface)
}

signature += argument_list(tfunction->get_arglist(), &pre, &post) + ")";
if (!send_part) {
signature += func_hint(tfunction->get_returntype());
}

return signature;
}

Expand Down Expand Up @@ -2771,6 +2810,7 @@ string t_py_generator::argument_list(t_struct* tstruct, vector<string>* pre, vec
result += ", ";
}
result += (*f_iter)->get_name();
result += arg_hint((*f_iter)->get_type());
}
if (post) {
for (s_iter = post->begin(); s_iter != post->end(); ++s_iter) {
Expand Down Expand Up @@ -2800,8 +2840,78 @@ string t_py_generator::type_name(t_type* ttype) {
return ttype->get_name();
}

string t_py_generator::arg_hint(t_type* type) {
if (gen_type_hints_) {
return ": " + type_to_py_type(type);
}

return "";
}

string t_py_generator::member_hint(t_type* type, t_field::e_req req) {
if (gen_type_hints_) {
if (req != t_field::T_REQUIRED) {
return ": typing.Optional[" + type_to_py_type(type) + "]";
Copy link
Member

Choose a reason for hiding this comment

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

i wonder if we can just generate | None if python version allows it

Copy link
Author

Choose a reason for hiding this comment

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

I believe this would work, but only as of 3.10 (which is EOL in a couple months.) Using typing.* and Union syntax allows us to target all of 3.5 to 3.12+

} else {
return ": " + type_to_py_type(type);
}
}

return "";
}

string t_py_generator::func_hint(t_type* type) {
if (gen_type_hints_) {
return " -> " + type_to_py_type(type);
}

return "";
}

/**
* Converts the parse type to a Python type
*/
string t_py_generator::type_to_py_type(t_type* type) {
type = get_true_type(type);

if (type->is_binary()) {
return "bytes";
}
if (type->is_base_type()) {
t_base_type::t_base tbase = ((t_base_type*)type)->get_base();
switch (tbase) {
case t_base_type::TYPE_VOID:
return "None";
case t_base_type::TYPE_STRING:
return "str";
case t_base_type::TYPE_BOOL:
return "bool";
case t_base_type::TYPE_I8:
case t_base_type::TYPE_I16:
case t_base_type::TYPE_I32:
case t_base_type::TYPE_I64:
return "int";
case t_base_type::TYPE_DOUBLE:
return "float";
case t_base_type::TYPE_UUID:
return "UUID";
}
} else if (type->is_enum() || type->is_struct() || type->is_xception()) {
return type_name(type);
} else if (type->is_map()) {
return "dict[" + type_to_py_type(((t_map*)type)->get_key_type()) + ", "
+ type_to_py_type(((t_map*)type)->get_val_type()) + "]";
} else if (type->is_set()) {
return "set[" + type_to_py_type(((t_set*)type)->get_elem_type()) + "]";
} else if (type->is_list()) {
return "list[" + type_to_py_type(((t_list*)type)->get_elem_type()) + "]";
}

throw "INVALID TYPE IN type_to_py_type: " + type->get_name();
}

/**
* Converts the parse type to a Python tyoe
* Converts the parse type to a Python type enum
*/
string t_py_generator::type_to_enum(t_type* type) {
type = get_true_type(type);
Expand All @@ -2825,6 +2935,8 @@ string t_py_generator::type_to_enum(t_type* type) {
return "TType.I64";
case t_base_type::TYPE_DOUBLE:
return "TType.DOUBLE";
case t_base_type::TYPE_UUID:
return "TType.UUID";
default:
throw "compiler error: unhandled type";
}
Expand Down Expand Up @@ -2883,7 +2995,6 @@ std::string t_py_generator::display_name() const {
return "Python";
}


THRIFT_REGISTER_GENERATOR(
py,
"Python",
Expand All @@ -2904,4 +3015,5 @@ THRIFT_REGISTER_GENERATOR(
" Package prefix for generated files.\n"
" old_style: Deprecated. Generate old-style classes.\n"
" enum: Generates Python's IntEnum, connects thrift to python enums. Python 3.4 and higher.\n"
" type_hints: Generate type hints and type checks in write method, including IntEnum generation.\n"
)
12 changes: 11 additions & 1 deletion test/py/Makefile.am
View file
Open in desktop
Expand Up @@ -52,7 +52,11 @@ thrift_gen = \
gen-py-enum/ThriftTest/__init__.py \
gen-py-enum/DebugProtoTest/__init__.py \
gen-py-enum/DoubleConstantsTest/__init__.py \
gen-py-enum/Recursive/__init__.py
gen-py-enum/Recursive/__init__.py \
gen-py-type_hints/ThriftTest/__init__.py \
gen-py-type_hints/DebugProtoTest/__init__.py \
gen-py-type_hints/DoubleConstantsTest/__init__.py \
gen-py-type_hints/Recursive/__init__.py

distdir:
$(MAKE) $(AM_MAKEFLAGS) distdir-am
Expand Down Expand Up @@ -119,6 +123,12 @@ gen-py-enum/%/__init__.py: ../%.thrift $(THRIFT)
&& $(THRIFT) --gen py:enum -out gen-py-enum ../v0.16/$(notdir $<) \
|| $(THRIFT) --gen py:enum -out gen-py-enum $<

gen-py-type_hints/%/__init__.py: ../%.thrift $(THRIFT)
test -d gen-py-type_hints || $(MKDIR_P) gen-py-type_hints
test ../$(notdir $<) \
&& $(THRIFT) --gen py:type_hints,enum -out gen-py-type_hints ../$(notdir $<) \
|| $(THRIFT) --gen py:type_hints,enum -out gen-py-type_hints $<

clean-local:
$(RM) -r build
find . -type f \( -iname "*.pyc" \) | xargs rm -f
Expand Down
2 changes: 1 addition & 1 deletion test/py/RunClientServer.py
View file
Open in desktop
Expand Up @@ -259,7 +259,7 @@ def main():
parser = OptionParser()
parser.add_option('--all', action="store_true", dest='all')
parser.add_option('--genpydirs', type='string', dest='genpydirs',
default='default,slots,oldstyle,no_utf8strings,dynamic,dynamicslots,enum',
default='default,slots,oldstyle,no_utf8strings,dynamic,dynamicslots,enum,type_hints',
help='directory extensions for generated code, used as suffixes for \"gen-py-*\" added sys.path for individual tests')
parser.add_option("--port", type="int", dest="port", default=9090,
help="port number for server to listen on")
Expand Down
4 changes: 4 additions & 0 deletions test/py/generate.cmake
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@ generate(${MY_PROJECT_DIR}/test/v0.16/ThriftTest.thrift py:no_utf8strings gen-py
generate(${MY_PROJECT_DIR}/test/v0.16/ThriftTest.thrift py:dynamic gen-py-dynamic)
generate(${MY_PROJECT_DIR}/test/v0.16/ThriftTest.thrift py:dynamic,slots gen-py-dynamicslots)
generate(${MY_PROJECT_DIR}/test/v0.16/ThriftTest.thrift py:enum gen-py-enum)
generate(${MY_PROJECT_DIR}/test/ThriftTest.thrift py:type_hints,enum gen-py-type_hints)

generate(${MY_PROJECT_DIR}/test/v0.16/DebugProtoTest.thrift py gen-py-default)
generate(${MY_PROJECT_DIR}/test/v0.16/DebugProtoTest.thrift py:slots gen-py-slots)
Expand All @@ -22,6 +23,7 @@ generate(${MY_PROJECT_DIR}/test/v0.16/DebugProtoTest.thrift py:no_utf8strings ge
generate(${MY_PROJECT_DIR}/test/v0.16/DebugProtoTest.thrift py:dynamic gen-py-dynamic)
generate(${MY_PROJECT_DIR}/test/v0.16/DebugProtoTest.thrift py:dynamic,slots gen-py-dynamicslots)
generate(${MY_PROJECT_DIR}/test/v0.16/DebugProtoTest.thrift py:enum gen-py-enum)
generate(${MY_PROJECT_DIR}/test/DebugProtoTest.thrift py:type_hints,enum gen-py-type_hints)

generate(${MY_PROJECT_DIR}/test/DoubleConstantsTest.thrift py gen-py-default)
generate(${MY_PROJECT_DIR}/test/DoubleConstantsTest.thrift py:slots gen-py-slots)
Expand All @@ -30,6 +32,7 @@ generate(${MY_PROJECT_DIR}/test/DoubleConstantsTest.thrift py:no_utf8strings gen
generate(${MY_PROJECT_DIR}/test/DoubleConstantsTest.thrift py:dynamic gen-py-dynamic)
generate(${MY_PROJECT_DIR}/test/DoubleConstantsTest.thrift py:dynamic,slots gen-py-dynamicslots)
generate(${MY_PROJECT_DIR}/test/DoubleConstantsTest.thrift py:enum gen-py-enum)
generate(${MY_PROJECT_DIR}/test/DoubleConstantsTest.thrift py:type_hints,enum gen-py-type_hints)

generate(${MY_PROJECT_DIR}/test/Recursive.thrift py gen-py-default)
generate(${MY_PROJECT_DIR}/test/Recursive.thrift py:slots gen-py-slots)
Expand All @@ -38,3 +41,4 @@ generate(${MY_PROJECT_DIR}/test/Recursive.thrift py:no_utf8strings gen-py-no_utf
generate(${MY_PROJECT_DIR}/test/Recursive.thrift py:dynamic gen-py-dynamic)
generate(${MY_PROJECT_DIR}/test/Recursive.thrift py:dynamic,slots gen-py-dynamicslots)
generate(${MY_PROJECT_DIR}/test/Recursive.thrift py:enum gen-py-enum)
generate(${MY_PROJECT_DIR}/test/Recursive.thrift py:type_hints,enum gen-py-type_hints)