Conversation
wholmgren
left a comment
wholmgren
left a comment
There was a problem hiding this comment.
Thanks, this is much easier for me to review now.
We need a much smaller sample file (it's currently 44k lines and 5 MB). I think only a day of data would be fine for the sample file.
Please also remove all of the code that you've commented out.
| if "Time reference" in line: | ||
| if "Universal time (UT)" in line: | ||
| tz_raw = 'UTC' | ||
| else: |
There was a problem hiding this comment.
Can these files ever have a time zone different from UTC or can they ever have no time zone information?
| name = file_csv.split('.')[0] | ||
|
|
||
| ## if file is on local drive | ||
| f = open(file_csv) |
There was a problem hiding this comment.
need to use a context manager to guarantee that the file will be closed.
|
|
||
|
|
||
|
|
||
| location = Location(lat, lon, name=name, altitude=alt, |
There was a problem hiding this comment.
Location is part of the higher level API, so we don't want to force people into it in a low level reader. Instead, build and return a dict. You can add a constructor method to Location that consumes that dict.
| tz=tz_loc) | ||
|
|
||
| #XXX | ||
| metadata = None |
|
|
||
| ## if file is on local drive | ||
| f = open(file_csv) | ||
| for line in f: |
There was a problem hiding this comment.
Need to add a way to stop the iteration once we reach the end of the header.
| # if line.startswith( "# Longitude"): | ||
| lon_line = line | ||
| lon = float(lon_line.split(':')[1]) | ||
| lon = float(line.split(':')[1]) |
| # latitude and longitude coordinates? | ||
| # http://stackoverflow.com/a/16086964 | ||
| # upstream: https://github.com/MrMinimal64/timezonefinder | ||
| from timezonefinder import TimezoneFinder |
|
|
||
| return tz | ||
|
|
||
| def localise_df(df_notz, tz_source_str='UTC', tz_target_str=None): |
There was a problem hiding this comment.
Why do we need this function? Seems more clear and robust to only call the pandas methods.
| Retrieve timezone | ||
| """ | ||
| if not name: | ||
| name = file_csv.split('.')[0] |
There was a problem hiding this comment.
This sounds fragile. Check out the os module for how to separate the file name from the full path. Next, are you trying to drop .csv from the name with the split? If so, do that explicitly.
|
|
||
| def read_maccrad_metadata(file_csv, name='maccrad'): | ||
| """ | ||
| Read metadata from commented lines of the file |
|
Still interested in the feature, but this PR would need to be reopened against the master branch. |
2 participants
a separate PR for MACC-RAD as requested in #274 (comment)
Discussion in #271