skyfielders / python-skyfield

Elegant astronomy for Python
MIT License
1.39k stars 209 forks source link

Download temporary filename for IERS EOP data incorrect in iokit.py #738

Closed aendie closed 2 years ago

aendie commented 2 years ago

A simple minor fix to a "cosmetic only" issue ...

Judging by the temporary filenames used to download ".bsp" ephemeris data where ".download" + optionally a unique number is appended to the filename, the unique temporary download filename for the IERS EOP data (finals2000A.all) forgets the filename entirely and downloads initially into the filename ".download" (where "finals2000A.all.download" would be correct).

Line 541 (from Skyfield 1.42) in iokit.py is incorrect:

iokit py snippet

It should be IMHO:

tempbase = tempname = path + filename + '.download'

(I've tested this in my alternate download code using the USNO website instead.)

brandon-rhodes commented 2 years ago

@aendie — I can't see that the download() routine has anything called filename by the time it reaches 541, so I think your suggestion would result in a NameError once the code reached the edited line. Could you double-check the source and, if you see filename defined somewhere, could you point out where? Thanks!

aendie commented 2 years ago

@brandon-rhodes -- you're correct. Nothing gets the filename in download(url, path, verbose=None, blocksize=128*1024, backup=False), however when you download any ephemeris (.bsp), it correctly appends ".download" to the filename, so those temporary filenames are correct, e.g. bsp421.bsp.download. My testing was only in the downloadUSNO(path, filename) that I wrote.

EDIT: I realize now that I was only testing my downloadUSNO() routine ... because the ftp.iers.org server is unreachable. It looks as if path in iokit.py does include the filename. I can only test this when the IERS server is alive again. So this is probably my mistake. My apologies (and I'll re-test this once the IERS server is running again).

aendie commented 2 years ago

@brandon-rhodes -- one additional point (if I may add it here): I've discovered that downloading any ephemeris (.bsp) file (using Skyfield, any version) is unreliable. It will probably download the full file 90% of the times, but it can hang before it reaches 100%. I also notice that when it hangs (if the download hangs), it is always near the end (typically above 95% downloaded). This suggests to me that the connection termination (above 95% downloaded) is at fault. It never hangs below 90% complete. Maybe this suggest to you where the problem probably lies. It's easy (but time-consuming) to test: repeatedly delete the ephemeris (.bsp) file you're using, so that Skyfield is forced to download anew. Ten attempts should be sufficient to reveal that Skyfield can hang while downloading. EDIT: Running tests just now I cannot get it to fail. Some days ago it was noticeably hanging. I guess this was due to network overload, or maybe the download server was at fault.

brandon-rhodes commented 2 years ago

Thank you for the "Edit", as I can't get it to fail either. As the Skyfield code doesn't seem to have any code paths that are sensitive to how much of the file has downloaded, I'm guessing it can't be Skyfield that's at fault here. Do let me know if you find out otherwise, however!