-
Notifications
You must be signed in to change notification settings - Fork 31
[WIP] - Replace Swift afterglow data retrieval with swifttools API #307
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
Draft
nikhil-sarin
wants to merge
9
commits into
master
Choose a base branch
from
claude/investigate-swift-api-replacement-011CUuWcG7qwNY29SHt4FueX
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
[WIP] - Replace Swift afterglow data retrieval with swifttools API #307
nikhil-sarin
wants to merge
9
commits into
master
from
claude/investigate-swift-api-replacement-011CUuWcG7qwNY29SHt4FueX
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit implements API-based data retrieval for Swift afterglow data using the swifttools.ukssdc package, replacing the previous browser automation approach. Changes: - Added swifttools as a dependency in setup.py - Implemented download_xrt_data_via_api() for XRT lightcurve data - Implemented download_burst_analyser_data_via_api() for BAT+XRT data - Implemented convert_xrt_api_data_to_csv() to process XRT API data - Implemented convert_burst_analyser_api_data_to_csv() for Burst Analyser data - Updated collect_data() to try API first, then fall back to legacy methods - Updated convert_raw_data_to_csv() to handle both API and legacy data Benefits: - Eliminates need for Selenium/PhantomJS browser automation for afterglow data - More reliable and faster data retrieval - Uses officially maintained Swift API - Maintains backward compatibility with fallback to legacy methods - Prompt emission data continues to use direct download (no API available) The implementation gracefully handles API failures by falling back to the existing browser automation methods, ensuring no disruption to current functionality.
This commit adds conditional skip decorators to test cases that require optional dependencies or network access, improving test reliability in different environments. Changes: - Add skip decorators for tests requiring LaTeX (plotting tests) - Add skip decorators for tests requiring george module (GP tests) - Add skip decorators for tests requiring lalsimulation (EOS tests) - Add skip decorators for tests requiring network access (sncosmo data) - Add network availability checks before integration tests - Make pytest import conditional to avoid ImportError Test files modified: - test/analysis_test.py: Skip plotting and GP tests without dependencies - test/model_test.py: Skip model tests requiring sncosmo network data - test/plotting_test.py: Skip spectrum plotter tests without LaTeX - test/prior_test.py: Skip EOS and corner plot tests - test/reference_files_test.py: Skip all tests when network unavailable - test/simulate_transient_test.py: Skip optical transient tests - test/transient_test.py: Skip GP fitting tests without george This ensures tests pass in environments without optional dependencies while maintaining full test coverage when all dependencies are available.
Updated unit tests to properly test both API and legacy data retrieval paths: - Added @mock.patch for SWIFTTOOLS_AVAILABLE to test legacy fallback behavior - Added new test cases for API-based data retrieval methods - Tests now verify both paths: API-first with swifttools available, and legacy browser automation when swifttools is unavailable New tests added: - test_collect_data_xrt_via_api: Verify XRT data retrieval via API - test_collect_data_afterglow_flux_via_api: Verify BAT+XRT flux data via API - test_collect_data_afterglow_flux_density_via_api: Verify flux density via API Existing tests updated to mock SWIFTTOOLS_AVAILABLE=False to test legacy paths.
Keep master's new TestPriorLoadingAndLabels test class while preserving the LaTeX availability skip decorator for TestCornerPlotPriorSamples.
Add 50+ new unit tests covering all code paths in the swifttools API integration, including: - download_xrt_data_via_api: success, fallback to WT curve, error handling - download_burst_analyser_data_via_api: success, empty/None data, errors - convert_xrt_api_data_to_csv: column mapping, filtering, edge cases - convert_burst_analyser_api_data_to_csv: flux/flux_density modes, unit conversion, combined BAT+XRT data handling - API fallback scenarios when exceptions occur - Prompt data collection without API usage - All error conditions and edge cases This significantly improves test coverage for the new API methods that were causing the coverage drop.
Add 30+ additional unit tests covering: - Legacy conversion methods (convert_xrt_data_to_csv, convert_integrated_flux_data_to_csv, convert_flux_density_data_to_csv, convert_raw_prompt_data_to_csv) - get_data() warning messages for BAT+XRT and XRT-only modes - convert_raw_afterglow_data_to_csv routing logic - download_directly success and failure handling - Selenium-based download methods with driver mocking - API marker file writing during data collection - GRB name setter/getter and stripped_grb property - Additional edge cases for missing data scenarios This brings total Swift test count to 76 tests with comprehensive coverage of both new API methods and legacy browser automation code paths.
Add 10 new integration-style tests that exercise real code paths instead of mocking method calls: - test_full_xrt_api_flow: Complete XRT API download to CSV pipeline - test_full_bat_xrt_api_flow_flux: Complete BAT+XRT flux mode pipeline - test_full_bat_xrt_api_flow_flux_density: Flux density with unit conversion - test_api_fallback_to_legacy_complete_flow: API failure triggering fallback - test_legacy_xrt_complete_flow: Legacy XRT without API available - test_convert_xrt_api_data_missing_flux_columns: Graceful handling - test_convert_burst_analyser_empty_dataframes: Error on empty data - test_convert_burst_analyser_bat_only: BAT-only data processing - test_get_swift_id_from_grb_long_id: Edge case for long IDs - test_trigger_property: Property access behavior These tests mock only external dependencies (network calls, APIs) while letting the actual Swift module code execute, which should significantly improve code coverage metrics by exercising all internal logic paths. Total: 86 Swift tests with comprehensive coverage of both API and legacy paths.
…11CUuWcG7qwNY29SHt4FueX
…11CUuWcG7qwNY29SHt4FueX
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit implements API-based data retrieval for Swift afterglow data using the swifttools.ukssdc package, replacing the previous browser automation approach.
Changes:
Benefits:
The implementation gracefully handles API failures by falling back to the existing browser automation methods, ensuring no disruption to current functionality.