pyuvsim icon indicating copy to clipboard operation
pyuvsim copied to clipboard

Fix check_kw handling and changelog errors

Open bhazelton opened this issue 9 months ago • 4 comments

Description

This PR fixes some errors introduced in recent PRs.

I discovered errors in how #503 was implemented, so I fixed them here.

I also discovered that there were mistakes in changelog updates in #503 and #519 where changes were recorded as going into version 1.4.0 rather than into the unreleased section. These are fixed here as well.

Motivation and Context

Types of changes

  • [x] 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)
  • [ ] Reference simulation update or replacement
  • [ ] Documentation change (documentation changes only)
  • [ ] Version change
  • [ ] Build or continuous integration change

Checklist:

For all pull requests:

  • [x] I have read the contribution guide.
  • [x] 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).
  • [x] All new and existing tests pass.
  • [ ] I have checked that I reproduce the reference simulations or if there are differences they are explained below (if appropriate). If there are changes that are correct, I will update the reference simulation files after this PR is merged.
  • [ ] I have checked (e.g., using the benchmarking tools) that this change does not significantly increase typical runtimes. If it does, I have included a justification in the comments on this PR.
  • [x] I have updated the CHANGELOG.

bhazelton avatar Feb 27 '25 00:02 bhazelton

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (24c2a87) to head (88c7c01). Report is 65 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #528   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         2025      2019    -6     
=========================================
- Hits          2025      2019    -6     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Feb 27 '25 00:02 codecov[bot]

hera_sim tests are failing for two reasons. One of them is that they use the check_kw argument for _complete_uvdata, so I've made a PR to fix that. The other however doesn't seem to be to do with hera_sim itself -- it's an error that telecfg is not a key in extra_keywords, and the error seems to be coming from a function in pyuvsim itself, so we should check that out.

I fixed the extra keywords problem, but if hera_sim is calling the (private) _complete_uvdata method then maybe I should just put the check_kw option back in. I took it out because I figured only trusted code was calling it.

bhazelton avatar Feb 27 '25 19:02 bhazelton

I fixed the extra keywords problem, but if hera_sim is calling the (private) _complete_uvdata method then maybe I should just put the check_kw option back in. I took it out because I figured only trusted code was calling it.

We understand that it's a private method, so API changes are allowed to happen. If it's made a public API, then more stability would be required, but I think it's fine. It's a simple fix on the hera-sim end.

steven-murray avatar Feb 27 '25 22:02 steven-murray

@steven-murray if you're good with this now can you re-approve?

bhazelton avatar Feb 27 '25 22:02 bhazelton