TUW-GEO / ismn

Readers for the data from the International Soil Moisture Network
https://ismn.earth/en/
MIT License
32 stars 21 forks source link

get_min_max_timestamp functions #24

Closed maawoo closed 3 years ago

maawoo commented 4 years ago

Hi there! I just noticed that get_min_max_timestamp_header_values, get_min_max_timestamp_ceop_sep and get_min_max_timestamp_ceop are all pretty much the same, except of the line _ = fid.readline() which is included in the header_values function. Seems like those functions are copy-pasted and have not been finished since then? Anyway, I'm working with header_value files and to actually get the min & max timestamps, I had to change the function a little bit and it now looks like this:

def get_min_max_timestamp_header_values(file_path):
    with io.open(file_path, mode='r', newline=None) as fid:
        lines = fid.readlines()
        first = lines[3]
        last = tail(fid)[0]

    min_date = datetime.strptime(first[:16], '%Y/%m/%d %H:%M')
    max_date = datetime.strptime(last[:16], '%Y/%m/%d %H:%M')

    return min_date, max_date

If I find some time, I can also test the other two functions and fix them.

Cheers, Marco

daberer commented 4 years ago

Hi Marco, you are right, get_min_max_timestamp_header_values is not working properly. However, if I apply your changes the tests no longer pass. We'll have to look into this, and get back to you. thanks!

daberer commented 4 years ago

Hi Marco, there has been a change in format for the header files (an extra line-break in the second row seems to be the problem). I would suggest we add a line to your code and then include it in the next release.

def get_min_max_timestamp_header_values(file_path):
    with io.open(file_path, mode='r', newline=None) as fid:
        lines = fid.readlines()
        if not lines[1].isspace():
        first = lines[1]
        else:
        first = lines[2]
        last = lines[-1]

    min_date = datetime.strptime(first[:16], '%Y/%m/%d %H:%M')
    max_date = datetime.strptime(last[:16], '%Y/%m/%d %H:%M')

    return min_date, max_date

Thanks again and please share more suggestions & improvements, when you find the time!

Cheers, Daniel

wpreimes commented 3 years ago

@daberer we can close this issue, right?

maawoo commented 3 years ago

@wpreimes seems to be fixed in #27, so I'll just close this 👍🏻