5
\$\begingroup\$

I get list JSON text from a webservice and list_of_columns is predefined, I need the output in that format. I am not sure about few things which are as follows

  1. I don't need "\n" towards the very end and this code seems sub-optimal to me but right now I can't think of any pythonic way to do it.

  2. My order for the code should be defined by list_of_columns. I don't know if this I have done is the optimal way to do it.

All in all my code does the job but I want to improve more. I suspect I could have used something that's already present that would have been much easier from what I have done. I am here for some criticism and learning :)

NOTE: I have used csv module to do the same but since separator had to be 2 characters long I had to drop that idea.

For formatting I tried using strftime but since dates in the data were less than 1900, I had to drop that idea. I think from what I have done with the date something better could have been done.

def read_json_text_write_text_file(json_text,file_name,list_of_columns,separator = "|~"):
    list_of_dictionaries = json.loads(json_text)

    with open(file_name, "w") as text_file:
        text_file.seek(0)
        for dictionary in list_of_dictionaries:
            for column in list_of_columns:
                if column in dictionary.keys():
                    if "DATE" in column:
                        text_file.write(convert_utc_to_est(dictionary[column]))
                    else:
                        text_file.write(str(dictionary[column]))
                if column != list_of_columns[-1]:
                    text_file.write(separator)
            if dictionary != list_of_dictionaries[-1]:
                text_file.write("\n")


def convert_utc_to_est(date_string):

    from_zone = dateutil.tz.gettz('UTC')
    to_zone = dateutil.tz.gettz('EST5EDT')

    utc = parser.parse(date_string)
    utc = utc.replace(tzinfo=from_zone)
    central = utc.astimezone(to_zone)
    # tzinfo is set to None else it would be present during the return
    central = central.replace(tzinfo=None)
    # This split is performed to remove the micro seconds part.
    return str(central).split(".")[0]
\$\endgroup\$
1
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Mast
    May 11, 2018 at 11:32

2 Answers 2

4
\$\begingroup\$

I am here for some criticism and learning :)

That's really a great attitude. Keep it up and you'll get far.

Handling the last item differently

You alluded to the issue of doing social treatment for the last item in a loop. There are two related relevant places in the posted code:

    for dictionary in list_of_dictionaries:
        for column in list_of_columns:
            # ...
            if column != list_of_columns[-1]:
                # special treatment 1
        if dictionary != list_of_dictionaries[-1]:
            # special treatment 2

Having a conditional statement that is executed for every item in a list, and you know it is false for all items except the last, is not pretty. It could be acceptable, if the condition is super fast, for example a simple comparison of two numbers. Here you are comparing strings and objects, which may not be super fast.

You could convert this to a simple numeric comparison by using a range loop, and then comparing indexes instead of objects.

You could also change the loops to iterate until the element before the end, and then do the special treatment for the last, outside the loop. That way you can eliminate the conditional statements.

Seeking in a file

When opening a file for writing with the w option only, there's no need to seek(0) in it, that will happen automatically, and the file will be rewritten.

Extracting the beginning of a string until some character

It's not efficient to use split for this. It will allocate a list to store the results, and scan until the end of the string, even though you only need the first part. It would be better to find the index of the dot, and then get a substring.

\$\endgroup\$
0
\$\begingroup\$

To remove the microseconds part the best move should be using replace in-built function in datetime module

The following will line with change

return str(central).split(".")[0]

To

central = central.replace(microsecond=0)
return str(central)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.