Added kwarg to convert_hdf5_to_fits to consistently trim too-long column names

[Stargrazer82301]

Discovered convert_hdf5_to_fits fails if the column names in .phot.hdf5 file are too long. These column names are typically set by the names of the files from MAST on which the photometry was done.

This fix adds an autotrim kwarg to the function (default to False).

This option, if column names are too long, finds the largest contiguous substring common to the column names that are too long for FITS key names. It then makes a shorter version of those column names. it first attempts to just remove - and _ characters from the column names; and after that, it lops off the beginning characters of the largest contiguous common substring (as these are most likely to be generic fluff). The shortened column name substring is then propagated to all column names that contain the original string.

[codecov]

Codecov Report

❌ Patch coverage is 7.69231% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.42%. Comparing base (b799489) to head (0c88ea7).
⚠️ Report is 70 commits behind head on master.

Files with missing lines	Patch %	Lines
beast/tools/convert_hdf5_to_fits.py	7.69%	36 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #842      +/-   ##
==========================================
- Coverage   40.55%   40.42%   -0.13%     
==========================================
  Files         107      107              
  Lines       10320    10358      +38     
==========================================
+ Hits         4185     4187       +2     
- Misses       6135     6171      +36     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Stargrazer82301]

Doc test fail from ACCESS, as with prior PR, and some ADS timeouts? Anyways, not at our end.

Unsure what the coverage issue is... Is that just because the new kwarg isn't True by default, and isn't in the tests?

[karllark]

Please see my comments. Just trying to get my head around this PR.

[karllark]

If this function will crash if this variable is false, why not make the default true? And issue a statement of what is happening? This sounds like a better solution than having a known crash.

[Stargrazer82301]

When this docstring means is that if the already-extisting st_file function will crash if the columns names are too long.

So shouldn't crash is this is false. Rather, if false, then the function replace_largest_common_substring simply won't get called. In that way, the behaviour of all existing code is unchanged. But now this exists for uses to enable if they want.

[karllark]

I am feeling dim right now. Where are these variables used? I don't see anything before the function ends, so it is not clear to me where the new set0_trim, etc. variables are used to update the column names.

[Stargrazer82301]

They're used in lines 91-99, to update the column names with the new, shorter, programmatically-generated column names produced by replace_largest_common_substring (if that's found to be necessary).

[karllark]

Unsure what the coverage issue is... Is that just because the new kwarg isn't True by default, and isn't in the tests?

This means the test coverage has decreased. Maybe because there is no test for kwarg? Or even many no test for this entire script.

[karllark]

Doc test fail from ACCESS, as with prior PR, and some ADS timeouts? Anyways, not at our end.

Yep. Might have to give up on having that URL in the docs.

[karllark]

Ping.