Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JP-3225: Gaia proper motions enabled #7614

Merged
merged 18 commits into from
Jun 30, 2023

Conversation

s-goldman
Copy link
Contributor

@s-goldman s-goldman commented Jun 21, 2023

Resolves JP-3225

This PR enables catalog positions to corrected for GAIA proper motions. The code follows the logic from romancal using an new epoch variable. The default for this variable is set to 2016.0 or no correction.

Checklist for maintainers

  • added entry in CHANGES.rst within the relevant release section
  • updated or added relevant tests
  • updated relevant documentation
  • added relevant milestone
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below.
    How to run regression tests on a PR
  • Make sure the JIRA ticket is resolved properly

@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (677c526) 76.62% compared to head (184ffcd) 76.62%.

❗ Current head 184ffcd differs from pull request most recent head 55fc080. Consider uploading reports for the commit 55fc080 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7614   +/-   ##
=======================================
  Coverage   76.62%   76.62%           
=======================================
  Files         456      456           
  Lines       36831    36833    +2     
=======================================
+ Hits        28221    28223    +2     
  Misses       8610     8610           
Flag Coverage Δ *Carryforward flag
nightly 77.43% <ø> (-0.01%) ⬇️ Carriedforward from 677c526

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
jwst/tweakreg/astrometric_utils.py 89.39% <100.00%> (+0.33%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

CHANGES.rst Outdated Show resolved Hide resolved
docs/jwst/tweakreg/README.rst Outdated Show resolved Hide resolved
jwst/tweakreg/astrometric_utils.py Outdated Show resolved Hide resolved
jwst/tweakreg/astrometric_utils.py Outdated Show resolved Hide resolved
@s-goldman s-goldman changed the title Gaia proper motions enabled. JP-3225: Gaia proper motions enabled Jun 22, 2023
@s-goldman s-goldman changed the title JP-3225: Gaia proper motions enabled [WIP] JP-3225: Gaia proper motions enabled Jun 23, 2023
@s-goldman s-goldman changed the title [WIP] JP-3225: Gaia proper motions enabled JP-3225: Gaia proper motions enabled Jun 26, 2023
@s-goldman

This comment was marked as outdated.

@nden
Copy link
Collaborator

nden commented Jun 26, 2023

It's for maintainers in general. However, it's also a reminder for those submitting PRs about what is expected. I think it's best if you check off items as you work through the PR and comments. It helps greatly the maintainer if those submitting PRs check these off and run regression tests with their PR.

CHANGES.rst Outdated Show resolved Hide resolved
docs/jwst/tweakreg/README.rst Outdated Show resolved Hide resolved
@hbushouse hbushouse added this to the Build 9.3 milestone Jun 29, 2023
@hbushouse
Copy link
Collaborator

jwst/tweakreg/astrometric_utils.py Outdated Show resolved Hide resolved
jwst/tweakreg/astrometric_utils.py Show resolved Hide resolved
jwst/tweakreg/astrometric_utils.py Outdated Show resolved Hide resolved
@hbushouse
Copy link
Collaborator

@s-goldman
Copy link
Contributor Author

s-goldman commented Jun 30, 2023

https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/771

Looks like 3 regression tests failed related to small difference (~1E-5) in generated results and the truth files. I can try to investigate this, but it may take me a little longer to get up to speed. Once the difference are confirmed to be benign, I can update the truth files.

@hbushouse
Copy link
Collaborator

https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/771

Looks like 3 regression tests failed related to small difference (~1E-5) in generated results and the truth files. I can try to investigate this, but it may take me a little longer to get up to speed. Once the difference are confirmed to be benign, I can update the truth files.

I think the small differences showing up are likely expected due to the inclusion of proper motion. I think we can merge the PR and do a detailed investigation of the new results once it's available in jwst/master.

@hbushouse hbushouse merged commit 1025a09 into spacetelescope:master Jun 30, 2023
@hbushouse hbushouse modified the milestones: Build 9.3, Build 10.0 Jul 3, 2023
@hbushouse hbushouse modified the milestones: Build 10.0, Build 9.3 Jul 12, 2023
@s-goldman s-goldman deleted the gaia_proper_motions branch August 14, 2023 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants