set FitsFactory.useHierarch to true

[olebole]

This is a forwarding followup of the short discussion in commit fd527d6 from @mbtaylor:

Change the static setting that determines whether the nom.tam.fits libraries recognise the ESO HIERARCH convention for hierarchical FITS keywords from false (which is how it is configured by default) to true.

This setting is required to make the hierarchical variant of the WideFits convention work (see uk.ac.starlink.fits.WideFits). There's no way in the nom.tam.fits API to configure HIERARCH-use on a per-call basis, so we have to modify the static setting.

This will affect other nom.tam.fits-dependent code using this library. An alternative would be to set this only when the hierarch convention is potentially or actually in use, but that gets messier,
since it would still affect other code running in the same JVM, but in less predictable ways.

So following this change, STIL FITS handling will pick up HIERARCH per-table headers which it previously ignored.

I looks that there is no real drawback when enabling this.

[ritchieGitHub]

This is not completely true ;) the settings can also be used on a per thread basis. that is almost compatible to per-call basis. This way the backward compatibility can be saved ;)
Would that not be sufficient?

[olebole]

Maybe @mbtaylor could comment on that. My primary interest to bring this up here is to keep Topcat fully usable with the standard nom.tam.fits package (for the Debian packaging). So, I could imagine that I have to patch uk.ac.starlink.fits in Debian to set this flag (which is OK for me as well).
What are the compatibility issues when this flag is set by default?

[ritchieGitHub]

the compatibility part is for @TomMcGlynn
you could also turn the flag on in a static part of the code.
..... hmmm just got an idea ... what about a property file where the defaults can be changed?
For example in /META-INF/nom-tam-fits-defaults.properties

[coveralls]

Coverage remained the same at 98.789% when pulling 63c7987 on olebole:useHierarch into d019419 on nom-tam-fits:master.

[mbtaylor]

The original topcat/starjava codebase is using an old version of nom.tam.fits that does not have the per-thread setting for useHierarch, so I can't put that in the upstream code. I could do something similar by setting the useHierarch value in a synchronized block where it's needed and restoring it to its previous behaviour at the end of the block, but I haven't done that.

The per-thread setting could in principle be used to address this in Ole's Debian version without changing the static default of that flag. However, you'd need to make sure the per-thread setting is set appropriately where required. TOPCAT uses lots of threads, including for things like loading FITS files, so you couldn't just do something like set it at the start of the application and assume that thread will be used for all subsequent access to FITS files.

At a quick look, I only see the useHierarch flag used in the nom.tam.fits.HeaderCard constructor, so it's a case of finding where the topcat/stilts code invokes that constructor and making sure that the per-thread useHierarch flag is set true before those invocations and restored to its previous value afterwards. I think that all these invocations are done from the static methods readHeader and addTrimmedHeader in uk.ac.starlink.fits.FitsConstants, so maybe you could do this by wrapping the bodies of those methods in a set/restore useHierarch block. But I haven't checked carefully.

[ritchieGitHub]

Sorry for the delay ... I was on a very long trip -> offline for a few Months ;-)
I will try to restart the comming weekend

[attipaci]

I have incorporated the fixes from this pull request into an unofficial 1.1.5.3-AK2 release on my fork.

Also, in case you have any further fixes, I'll plan to keep regularly integrating pull requests made to the base repo, at least until the base repo is actively mainained again. (I could no longer bear that so many good fixes were unheeded here...)

[olebole]

@attipaci Whow: this sounds that this repo is back to life, right? This is very appreciated. Thank you so much for your activity! 💯