WorldWideTelescope / toasty

Break large images into "tile pyramids", with a focus on the all-sky TOAST format.
https://toasty.readthedocs.io/
MIT License
0 stars 4 forks source link

Hipsgen #69

Closed imbasimba closed 2 years ago

imbasimba commented 2 years ago
codecov[bot] commented 2 years ago

Codecov Report

Merging #69 (f84f5ab) into master (52ab512) will decrease coverage by 1.73%. The diff coverage is 56.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
- Coverage   75.37%   73.63%   -1.74%     
==========================================
  Files          22       23       +1     
  Lines        3098     3289     +191     
==========================================
+ Hits         2335     2422      +87     
- Misses        763      867     +104     
Impacted Files Coverage Δ
toasty/fits_tiler.py 49.30% <49.30%> (ø)
toasty/collection.py 56.57% <55.55%> (-1.28%) :arrow_down:
toasty/cli.py 80.68% <68.11%> (-2.35%) :arrow_down:
toasty/__init__.py 100.00% <100.00%> (+10.71%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 52ab512...f84f5ab. Read the comment docs.

pkgw commented 2 years ago

@imbasimba OK, I think the main blocker on this is going to be the Astropy caching issue that we discussed. Have you been able to gather any more clues about what might be going on? As expected, I can't reproduce this on Linux. I'll try booting into Windows to see.

pkgw commented 2 years ago

Maybe something to do with this mis-firing?

https://github.com/astropy/astropy/blob/523b73e250ed2c86909570ca3c2862464c928abe/astropy/utils/data.py#L1418

edit: No, that only kicks in if the file can't be placed into the cache, which would result in other errors.

imbasimba commented 2 years ago

I've tested it on another Windows machine, and it happened there too

pkgw commented 2 years ago

@imbasimba I was just updating my comment to say that I can't reproduce the issue on my Windows partition. Argh!

imbasimba commented 2 years ago

Very curious...

imbasimba commented 2 years ago

I'm testing a bit not and I've started to suspect the hipsgen.jar doing something weird. The cache is filled correctly. Then I run java -jar [cache_path]/contents -> content is deleted

imbasimba commented 2 years ago

It is the antivirus!

imbasimba commented 2 years ago

Yup, it is definitely Avast. I've turned off silent mode now and I get some big popups when trying to run hipsgen. It does not allow java -jar on files that do not have the .jar extension. If I rename it to contents.jar avast does not interfere. image image

pkgw commented 2 years ago

As far as I can tell, the astropy caching system does not allow us to affect the extension of the cached filename. (Which I would consider a bug, since there are other cases where it is important to have the extension match someone else's expectations.) So I think that the most robust approach would be copy the file to a toasty-specific per-user cache location? Or how about: copy/symlink into the temporary directory used to hold the input files, so that we don't have to worry about maintaining our own cache across invocations. I think there's still value in using the Astropy file-downloading tools since they take care of a lot of generic things like proper SSL setup, etc.

Also: great detective work!

imbasimba commented 2 years ago

That a great idea! I'll go for the symlink in the temporary input folder 👍

pkgw commented 2 years ago

@imbasimba Then I was wondering whether Avast would follow the symlink and still be mad ... but if so, we can do a copy instead.

I'm about to push a fairly major update that fixes the various problems that I ran into. It also adds a toasty view command that is intended to be a magical "view my file!" command.

pkgw commented 2 years ago

OK, many changes:

imbasimba commented 2 years ago

@pkgw Avast was not so easily fooled... I guess a copy in the input directory will have to do.

pkgw commented 2 years ago

@imbasimba Great, nice explanatory comment too. Are you able to take a look at whether toasty view works well for you and your collection of sample files? I did some light testing but don't have a very large FITS collection to try out right now.

imbasimba commented 2 years ago

Amazing! Windows defender reacted with a popup, but clicking allow on that one makes this run smoothly.

The test suite seems to be mostly viewable. The problematic files are the same as before, and I'll launch a proper investigation next week. Here is the summary: