Bug fix for reading ms caltables with multiple times

[rlbyrne]

Description

Changes the way times are indexed when reading ms caltables, fixing a bug.

Motivation and Context

The bug affects reading of ms caltables with multiple closely-spaced times. A dictionary is constructed mapping the times in the caltable to those in the UVCal object. Distinct but closely-spaced times can map to the same output time. The mapping used the index of the distinct input times, not the output times, and could therefore produced an indexing error.

Closes #1648

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation change (documentation changes only)
• Version change
• Build or continuous integration change
• Other

Checklist:

• I have read the contribution guide.
• My code follows the code style of this project.

Bug fix checklist:

• My fix includes a new test that breaks as a result of the bug (if possible).
• I have updated the CHANGELOG.

[bhazelton]

This is great, thank you @rlbyrne ! We're having a problem with our CI right this minute because of a numpy update. As soon as we get a fix in we'll get you to rebase this so we can get it in. Sorry for the delay!

[bhazelton]

Ok, the CI issues have been fixed on the main branch. Can you please rebase?

[rlbyrne]

Done!

[bhazelton]

@rlbyrne I think we should definitely update the changelog and seriously consider adding a test that errors due to this bug.

[kartographer]

This looks great, thanks for catching this @rlbyrne - can you just add a test? The SMA gains files should already have multiple time stamps, and should be easy to read in.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.93%. Comparing base (2be64e0) to head (eea49d1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1653   +/-   ##
=======================================
  Coverage   99.93%   99.93%           
=======================================
  Files          67       67           
  Lines       22689    22689           
=======================================
  Hits        22675    22675           
  Misses         14       14           

☔ 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.

[rlbyrne]

@kartographer I'll need to make a new datafile to test this, because the bug only occurs when the timesteps are really close together (within the tolerance to be treated as a single time). Where is the test data stored?

[bhazelton]

@kartographer I'll need to make a new datafile to test this, because the bug only occurs when the timesteps are really close together (within the tolerance to be treated as a single time). Where is the test data stored?

The test data are stored in a separate repo now, I wrote up the process for updating them here: https://pyuvdata.readthedocs.io/en/latest/developer_docs.html

Please let me know if anything is unclear there, I think you might be the first person other than me to go through the process.

[bhazelton]

@kartographer I'll need to make a new datafile to test this, because the bug only occurs when the timesteps are really close together (within the tolerance to be treated as a single time). Where is the test data stored?

Another approach that might work (not sure if it would for this) is to read in some existing test data and modify it to have the characteristic you need for testing and then write it back out.

[rlbyrne]

@bhazelton I think this PR is done, but I'm not sure how to parse the failing tests. Could you take a look when you get the chance?

[bhazelton]

@bhazelton I think this PR is done, but I'm not sure how to parse the failing tests. Could you take a look when you get the chance?

Sigh, it looks like a new setuptools_scm version was just released that's generating a new warning. I haven't figured out what the issue is yet, but we can ignore warnings tests failures for now.