You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Jonathan Ellis (JIRA)" <ji...@apache.org> on 2008/12/24 18:26:44 UTC
[jira] Created: (THRIFT-242) Python struct constructors are clunky
and error-prone
Python struct constructors are clunky and error-prone
-----------------------------------------------------
Key: THRIFT-242
URL: https://issues.apache.org/jira/browse/THRIFT-242
Project: Thrift
Issue Type: Improvement
Components: Compiler (Python)
Reporter: Jonathan Ellis
The python constructors are clunky and unnecessarily difficult to use. Instead of
Cls(value1, value2, ...)
you must write
Cls({'arg1': value1, 'arg2': value2})
(or use the dict constructor instead of literal notation).
Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by Terry Jones <te...@jon.es>.
Sorry, wasn't thinking. This
def func(x=_default):
if x = _default:
x = DEFAULT
should say
def func(x=_default):
if x is _default:
x = DEFAULT
Terry
Re: [jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by Terry Jones <te...@jon.es>.
>>>>> "David" == David Reiss (JIRA) <ji...@apache.org> writes:
David> And it prints 1, 0. I think this needs to be avoided. The two best
David> solutions I can come up with are...
David> 1/ Default everything (at least non-scalars) to None, and make the
David> body of the __init__ say "if foo is None: foo = DEFAULT". The
David> downside is that it becomes impossible to override a default value
David> with None (in the constructor).
David> 2/ Take **kwargs. The downside is that it is less self-documenting.
David> Also, I think we would want to write code to throw an exception if
David> someone specified an invalid field.
David> Thoughts?
A simple way around this is to write
_default = object()
def func(x=_default):
if x = _default:
x = DEFAULT
You can use the same _default object for all args of this type in the same
file. This is a fairly well-known approach to this problem. The only
possible downside is someone deliberately reaching into the file to pass
_default as an arg, but in that case they get what they asked for...
Terry
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659606#action_12659606 ]
Jonathan Ellis commented on THRIFT-242:
---------------------------------------
That's true, but since the code is autogenerated anyway, there's no reason not to put the exact parameter names in for human readability.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Todd Lipcon (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659563#action_12659563 ]
Todd Lipcon commented on THRIFT-242:
------------------------------------
+1 on the general intent, but a couple questions:
1) Why is there no d argument in the case that there are no members? This seems like it would break back compat if someone was passing in {} before
2) With a default value of d={}, the loop checking for members in d will always execute. Why not set d=None and then check "if d != None"? This seems to me to be slightly less error prone since (a) passing a non-dictionary type will end up with an error instead of being ignored, and (b) you could then pass in any type that has a dictionary-like behavior (eg defaultdict)
3) There's a small typo: "d-handing" instead of "d-handling"
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Mark Slee (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12664289#action_12664289 ]
Mark Slee commented on THRIFT-242:
----------------------------------
Should we close this issue? Looks done to me, we just updated the website to reflect the change.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Assignee: Esteve Fernandez
> Attachments: init.patch, thrift-242_no_d_argument.patch, thrift-242_v2_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659572#action_12659572 ]
Jonathan Ellis commented on THRIFT-242:
---------------------------------------
I am happy to submit a d-less patch if The Powers That Be want one sooner rather than later. (Who are The Powers here? :)
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Will Pierce (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659601#action_12659601 ]
Will Pierce commented on THRIFT-242:
------------------------------------
Removing the extra dict to thrift constructors will improve both performance and readability in my python apps.
Another way to do this might be to use the built in kwargs feature:
def __init__(self, **d):
# start of constructor
# initialize self by using d.get(NAME, default) for each NAMEd element? (maybe default is None?)
justa thought!
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Will Pierce (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659633#action_12659633 ]
Will Pierce commented on THRIFT-242:
------------------------------------
You're right, and explicit argument naming will be far more usable for introspecting code.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12662563#action_12662563 ]
David Reiss commented on THRIFT-242:
------------------------------------
Hey, there is one thing that concerns me about this patch, regarding the handling of container and structure fields with default values. For an example, look at test/DebugProtoTest.thrift, the OneOfEach structure, the i16_list field. The generated Python code looks like...
{code}
def __init__(self, im_true=None, im_false=None, a_bite=200, integer16=33000, integer32=None, integer64=10000000000, double_precision=None, some_characters=None, zomg_unicode=None, what_who=None, base64=None, byte_list=[
1,
2,
3,
], i16_list=[
1,
2,
3,
], i64_list=[
1,
2,
3,
],):
{code}
What this means is that all OneOfEachs constructed without specifying i16_list will have a reference to the same list (Python default args are only evaluated at definition time, not call time). So I wrote this test program...
{code}
#!/usr/bin/env python
import sys
sys.path.append('./gen-py')
from DebugProtoTest.ttypes import OneOfEach
ooe1 = OneOfEach()
ooe2 = OneOfEach()
print ooe2.byte_list[0]
ooe1.byte_list[0] = 0
print ooe2.byte_list[0]
{code}
And it prints 1, 0. I think this needs to be avoided. The two best solutions I can come up with are...
1/ Default everything (at least non-scalars) to None, and make the body of the __init__ say "if foo is None: foo = DEFAULT". The downside is that it becomes impossible to override a default value with None (in the constructor).
2/ Take **kwargs. The downside is that it is less self-documenting. Also, I think we would want to write code to throw an exception if someone specified an invalid field.
Thoughts?
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-242) Python struct constructors are clunky
and error-prone
Posted by "Esteve Fernandez (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Esteve Fernandez updated THRIFT-242:
------------------------------------
Attachment: thrift-242_v2_no_d_argument.patch
Updated patch, constructors now take default values from thrift_spec and use the "field is value" trick.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch, thrift-242_v2_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660886#action_12660886 ]
David Reiss commented on THRIFT-242:
------------------------------------
I'd be interested to hear what Ben (Maurer) thinks of this, since ReCaptcha is one of the biggest Python/Thrift users.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12662571#action_12662571 ]
David Reiss commented on THRIFT-242:
------------------------------------
This suggestion was sent to the dev list. I like it.
A simple way around this is to write
{code}
_default = object()
def func(x=_default):
if x is _default:
x = DEFAULT
{code}
You can use the same _default object for all args of this type in the same
file. This is a fairly well-known approach to this problem. The only
possible downside is someone deliberately reaching into the file to pass
_default as an arg, but in that case they get what they asked for...
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Esteve Fernandez (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12663732#action_12663732 ]
Esteve Fernandez commented on THRIFT-242:
-----------------------------------------
Could someone apply the patch? Ben gave his approval on removing the d argument and
David's concern on mutable arguments is already solved in the last version of the patch.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch, thrift-242_v2_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Ben Maurer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12661450#action_12661450 ]
Ben Maurer commented on THRIFT-242:
-----------------------------------
I'm fine with removing the dictionary argument completely. Using a dictionary doesn't really help you do a copy constructor, and if you're going to specify fields, I think it's a better practice to not do that in a constructor.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-242) Python struct constructors are clunky
and error-prone
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated THRIFT-242:
----------------------------------
Attachment: init.patch
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-242) Python struct constructors are clunky
and error-prone
Posted by "Esteve Fernandez (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Esteve Fernandez updated THRIFT-242:
------------------------------------
Attachment: thrift-242_no_d_argument.patch
Patch that builds constructors without the d argument. Unit tests updated as well.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12664292#action_12664292 ]
David Reiss commented on THRIFT-242:
------------------------------------
It is marked resolved.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Assignee: Esteve Fernandez
> Attachments: init.patch, thrift-242_no_d_argument.patch, thrift-242_v2_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-242) Python struct constructors are clunky
and error-prone
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Reiss resolved THRIFT-242.
--------------------------------
Resolution: Fixed
Assignee: Esteve Fernandez
Committed the fix as r734536. I had to make one additional small change to render_constant_value for constant structures.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Assignee: Esteve Fernandez
> Attachments: init.patch, thrift-242_no_d_argument.patch, thrift-242_v2_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Esteve Fernandez (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12662645#action_12662645 ]
Esteve Fernandez commented on THRIFT-242:
-----------------------------------------
Oops, I forgot that. In any case, I would prefer to define defaults in thrift_spec, rather than using a sentinel. I'll upload a new patch by the end of the day.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659569#action_12659569 ]
Jonathan Ellis commented on THRIFT-242:
---------------------------------------
1) True, but since this addresses a special case of the "having the constructor silently do nothing if nonsense is passed" problem, I think it's worth causing (very) minor trouble if someone is actually explicitly passing {} -- or worse, a non-empty dict or any other value, all of which have the same effect! Which illustrates the problem. Basically, if they are doing any of those things, it is an error, and we are not doing them a favor by preserving the existing behavior.
(Obviously we also break back-compat for any parameters named "d.")
2) This is a stylistic preference; IMO, it's more clear (for users of help() as well as code divers) to have d be an empty instance of the type it expects, but certainly there is a lot of Python code out there that does things the way you suggest. (You didn't bring up performance as an argument, but for the record, the extra if may be more or less performant depending on what the common case is, so I think that is a wash.) I don't much care here since ultimately I want the d argument to go away. :)
3) Good catch.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660898#action_12660898 ]
Jonathan Ellis commented on THRIFT-242:
---------------------------------------
Note that Esteve's patch does not create an __init__ method at all when no arguments are possible. This is good python behavior. +1 for Esteve's patch.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch, thrift-242_no_d_argument.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-242) Python struct constructors are
clunky and error-prone
Posted by "Esteve Fernandez (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659568#action_12659568 ]
Esteve Fernandez commented on THRIFT-242:
-----------------------------------------
+1 However, I think backwards compatibility cannot be achieved completely. What happens if a structure has a member called "d"?
I would propose an even more dramatic change and drop the current behavior. The latest release is from March (20080411) and other language libraries have already changed their API, so I think backwards compatibility can be sacrificed in favor of correctness.
> Python struct constructors are clunky and error-prone
> -----------------------------------------------------
>
> Key: THRIFT-242
> URL: https://issues.apache.org/jira/browse/THRIFT-242
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Reporter: Jonathan Ellis
> Attachments: init.patch
>
>
> The python constructors are clunky and unnecessarily difficult to use. Instead of
> Cls(value1, value2, ...)
> you must write
> Cls({'arg1': value1, 'arg2': value2})
> (or use the dict constructor instead of literal notation).
> Additionally, having the constructor silently do nothing if a non-dict is passed in (an object of the type being created would be another reasonable guess) or if an argument is mis-spelled causes users unnecessary trouble debugging.
> Since removing the d argument entirely would be backwards-incompatible, the attached patch keeps it, but puts real keyword parameters in for forwards-compatibility. Thus, old code will work with this patch, but new code can be written using keyword args that will still work when the d argument is removed in a release that allows backwards-incompatibility. (Removing the d argument is desireable because otherwise positional arguments can't be used.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.