Fixed issue with single-item lists rendering incorrectly. Fixed all naming conventions and coding standards to adhere to PEP8. #17
Conversation
…aming conventions and coding standards to adhere to PEP8.
|
I support this patch, it's much cleaner and adheres mostly to PEP8. I wonder what you meant with
.. can't find it in the code. |
| convertedOutput += '</td></tr>' | ||
| converted_output += '<tr><td>' | ||
| converted_output += '</td><td>'.join([markup(list_entry[column_header]) for column_header in | ||
| column_headers]) |
There was a problem hiding this comment.
how about
converted_output += '</td><td>'.join([
markup(list_entry[column_header])
for column_header in column_headers
])
I like this style of closing brackets better, I find it much more readable.
| if len(inputtedJson) < 2: | ||
| return None | ||
| if not isinstance(inputtedJson[0], dict): | ||
| if not inputted_json or not isinstance(inputted_json[0], dict): |
There was a problem hiding this comment.
@softvar here is the functional change. Apologies it is hard to find amongst all the PEP8 changes - I should have made two separate commits. The issue was with the len(inputtedJson) < 2, whereas it should have been 1 to deal with the edge case. The final line is a nicer way to achieve the desired effect.
There was a problem hiding this comment.
Ah so you want lists to always follow the table style, even if only one item is present. Makes sense I think for consistency. The old way just created multiple two-column rows right?
There was a problem hiding this comment.
@lekic Ah, now I get that line :)
The change ideally looks good.
Would be great if you could add a screenshot comparing the changes. This way, it helps in keeping track of changes.
Thanks for the patch. 👍
Will be merging it for sure after this.
| @@ -78,88 +78,81 @@ def columnHeadersFromListOfDicts(self, inputtedJson): | |||
| ----------------------------- | |||
|
|
|||
| @contributed by: @muellermichel | |||
| ''' | |||
| """ | |||
|
|
|||
There was a problem hiding this comment.
I recommend adding an exception here when inputted_json is not a list. Actually inputted_json would better be renamed to input_list. The old code would throw an exception in most cases because len wouldn't exist, however a graceful error would be better for code stability in later iterations.
|
Thanks @lekic for the awesome work. |
|
It's there in v1.0.3 now -> https://pypi.python.org/pypi/json2html |
The changes are predominately formatting and conventions, with a minor change in the header generation section that allows for correct handling of one-item lists.