Repository navigation
feat: refresh F10.7 and ap data handling with partial data support - #74
Conversation
| """ | ||
| dates = np.asarray(dates, dtype=np.datetime64) | ||
| data = _DATA or _load_f107_ap_data() | ||
| data = _DATA or _load_f107_ap_data(dates[-1]) |
There was a problem hiding this comment.
If we have a long running process with _DATA already loaded, then this won't update the data for us. I think the logic should probably be a new if block here instead after the original loading potentially?
| data = _DATA or _load_f107_ap_data(dates[-1]) | |
| # Load/download data right away. | |
| data = _DATA or _load_f107_ap_data() | |
| if dates[-1] > data["dates"][-1]: | |
| data = _new_function_to_download_recent_data() |
|
|
||
| # Check if the file needs to be refreshed after parsing | ||
| if last_obs_date is not None: | ||
| file_mod_time = datetime.fromtimestamp(os.path.getmtime(_F107_AP_PATH)) |
There was a problem hiding this comment.
Do we need file modification time? I think we should just leave that out and only rely on the requested date being greater than the observations we have.
| if ",OBS," in line: | ||
| # Capture last observed date | ||
| last_obs_date = np.datetime64(line.split(",")[0]) |
There was a problem hiding this comment.
Can we get this from the _DATA numpy arrays? Maybe warn_data contains this information already and we could do something like (have not tried this) _DATA["dates"][~_DATA["warn_data"]][-1].
| writer.writeheader() | ||
| writer.writerows(sorted_data) | ||
|
|
||
| os.remove(_PARTIAL_F107_AP_PATH) |
There was a problem hiding this comment.
Be wary of this because if there is any download error above, this file will be left hanging around.
A few quick thoughts:
- Do we need to save it to disk, or could you store it in a BytesIO object in memory instead?
- Would it make sense to save
_DATAarrays withnp.savez()instead of the csv files for easier manipulation? (Then you wouldn't need to mess with the dictreader/writer. - Honestly, now that I'm thinking about it more, is this just more headache than it is worth and we should just use your logic below and re-download the full file for the simplest case. More bandwidth, but at least we are getting the full file built for us rather than building it from bits and pieces.
|
Hi @greglucas, I've pushed another commit based on your suggestions.
Let me know what you think. |
| """ | ||
| dates = np.asarray(dates, dtype=np.datetime64) | ||
| data = _DATA or _load_f107_ap_data() | ||
| if dates[-1] > data["dates"][~np.repeat(data["warn_data"], 8)][-1]: |
There was a problem hiding this comment.
If someone didn't sort their inputs/request, we should use a max here I think. Could you also add a quick comment about what this if statement is doing.
| if dates[-1] > data["dates"][~np.repeat(data["warn_data"], 8)][-1]: | |
| # If our requested data time is after the cached values we have, go and download a new file | |
| # to refresh the local file cache | |
| if dates.max() > data["dates"][~np.repeat(data["warn_data"], 8)][-1]: |
There was a problem hiding this comment.
Thanks @greglucas, I've commented the date comparison check
|
@mananapr I pushed a commit up to your branch to try and fix up some of the failing tests. Unfortunately, it was more involved than I hoped, so there is a bit more work there than anticipated. It also apparently didn't fix the issue :-/ But it does work locally for me. Feel free to drop that commit if you don't like it and continue on in a different way to try and get the tests to pass too. |
Ah, I totally missed that. Will look into the changes, thanks! |
f12ec35 to
9cdae71
Compare
Add tests to make sure the downloads are triggered appropriately. Change the condition to only refresh if the data being requested is before the current time AND after the last date in the file. Co-authored-by: Manan Singh <manan.singh@digantara.co.in>
9cdae71 to
634a4e4
Compare
|
I reworked some of the file download utilities recently so this needed to be updated. I made the changes and force-pushed to your branch leaving you as a co-author on the commit. Thanks for this addition! |
Refresh F10.7 and ap data handling with partial data support and automatic updates
Closes #69