-
Notifications
You must be signed in to change notification settings - Fork 17
New function to get TESS sectors for a position and cutout size #168
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #168 +/- ##
==========================================
+ Coverage 96.29% 96.36% +0.07%
==========================================
Files 17 17
Lines 1810 1818 +8
==========================================
+ Hits 1743 1752 +9
+ Misses 67 66 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new get_tess_sectors function that returns TESS sector information for sectors whose footprints overlap with given sky coordinates and a cutout size. The implementation refactors internal helper methods to be standalone functions, enabling them to be reused by the new public API function.
Key Changes:
- Introduced
get_tess_sectorsfunction to query TESS sectors by coordinates and cutout size - Refactored internal helper methods (
_extract_sequence_information,_create_sequence_list,_get_files_from_cone_results) from instance methods to module-level functions - Converted instance variables to class constants (
ARCSEC_PER_PX,S3_FOOTPRINT_CACHE,S3_BASE_FILE_PATH)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| astrocut/tess_footprint_cutout.py | Added get_tess_sectors function and refactored helper methods to module-level functions; converted instance variables to class constants |
| astrocut/init.py | Exported new get_tess_sectors function in public API |
| astrocut/tests/test_tess_footprint_cutout.py | Added test coverage for get_tess_sectors and updated existing tests to use refactored functions |
| astrocut/footprint_cutout.py | Removed abstract method and instance variables that are no longer needed in base class |
| CHANGES.rst | Documented new feature in changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_get_tess_sectors(coordinates): | ||
| """Test that get_tess_sectors returns sector list""" | ||
| sector_table = get_tess_sectors(coordinates, 0) | ||
| assert isinstance(sector_table, Table) | ||
| assert 'sectorName' in sector_table.colnames | ||
| assert 'sector' in sector_table.colnames | ||
| assert 'camera' in sector_table.colnames | ||
| assert 'ccd' in sector_table.colnames | ||
| assert len(sector_table) >= 7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only tests cutout_size=0
Can we add some parametrized tests covering:
- cutout_size as int/Quantity/array
- coordinates that match no sectors (are there any?)
- invalid coordinates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! For the case that coordinates don't match any sectors, I used a patch on ra_dec_crossmatch.
falkben
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, just minor request to add a bit more tests
Added
get_tess_sectorsfunction to return TESS sector information for sectors whose footprints overlap with the given sky coordinates and cutout size.If we want
astroquery.mast.Tesscutto be able to use astrocut directly, we will need to support theget_sectorsfunction. This PR will help to accomplish that.