Refactor orbital tracking with vectorized methods, bug fixes and improved utilities

[JaskRendix]

PR improves the orbital tracking module and related utilities.

• fixed a bug in the main script where observer longitude and latitude were being converted to radians before being passed to get_observer_look, which already performs this conversion internally, coordinates are now passed in degrees as expected, and conversion is handled inside the method

• refactored the get_observer_look function: split the logic into two helper functions: ecef_to_topocentric and compute_azimuth_elevation, this isolates coordinate transformations and makes the azimuth/elevation computation easier to test and reuse, pytest coverage added

• refactored _get_root and _get_max_parab: _get_root now includes a check for valid root bracketing and uses xtol and rtol for better convergence control. _get_max_parab now includes a fallback for flat functions, a maximum iteration limit, and improved numerical stability to avoid divergence and handle edge cases.

• cleaned up test_orbital: ran isort and black for formatting and added new pytest coverage for all newly introduced vectorized methods and utilities

• implemented find_aos and find_aol methods for detecting Acquisition of Signal and Loss of Signal events based on observer location and elevation mask; includes robust horizon-crossing logic and full pytest coverage

• improved get_last_an_time logic by refactoring shared node detection into a private _find_last_node_time method, which uses direct geometric analysis of the satellite’s Z-position and velocity to identify node crossings; this approach is distinct from orbit-number-based equatorial crossing methods and supports both ascending and descending detection, added get_last_dn_time to retrieve the last descending node, addressing feature request #95; includes parameterized pytest coverage for velocity sign, node classification, and temporal accuracy

• [x] Closes #95

• [x] Tests added

• [x] Fully documented

[codecov[bot]]

Codecov Report

:x: Patch coverage is 97.97297% with 9 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 91.06%. Comparing base (c9e2909) to head (6d2b779). :warning: Report is 16 commits behind head on main.

Files with missing lines	Patch %	Lines
pyorbital/orbital.py	94.92%	7 Missing :warning:
pyorbital/tests/test_orbital.py	98.88%	2 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #206      +/-   ##
==========================================
+ Coverage   89.78%   91.06%   +1.27%     
==========================================
  Files          17       19       +2     
  Lines        2809     3289     +480     
==========================================
+ Hits         2522     2995     +473     
- Misses        287      294       +7     

Flag	Coverage Δ
unittests	91.06% <97.97%> (+1.27%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[coveralls]

coverage: 91.024% (+0.6%) from 90.399% when pulling 6d2b77945d7edebf1c1a03aea3be77190eeea6c1 on JaskRendix:vectorized into 10467c05640a69a04a6c7f65ded92dc2cd98a7f7 on pytroll:main.

[mraspaud]

I'm pretty sure get_lonlatalt at least already support arrays as input, what is get_lonlatalt_vectorized offering more?

[JaskRendix]

@mraspaud absolutely nothing, I'm going to remove these

edit:

I’ve removed all of the _vectorized wrapper methods. While they offered an array-friendly API, they didn't provide true performance. I'm convinced that the SGP4 calculation is the real performance constraint and fixing it there will automatically make all public methods correctly array-aware.

[JaskRendix]

@djhoese I usually run mypy --strict, but it throws a ton of errors. Do you use a specific mypy command? If not, I can just strip out the typing so it matches the rest of the code, no big deal.

I checked and: get_lonlatalt still returns (float,float,float) or a tuple of np.ndarray's if vectorized get_last_an_time still returns a single time object (np.datetime64) _elevation still returns a single float _elevation_inv still returns a single float _get_root still returns a single float _get_max_parab still returns a single float both get_observer_look still returns a tuple of two floats

_find_single_crossing, find_aos, find_aol, ecef_to_topocentric and compute_azimuth_elevation are new

I'm going to run mypy less strict soon

[djhoese]

Typically I just go with mypy <pkgdir>/, but pyorbital is well behind other pytroll packages on linting/typing standards so we don't have anything existing setup.

[JaskRendix]

@djhoese yeah, I created 2 typehints, but I have fixed both

pyorbital/orbital.py:458: error: Argument 1 to "__call__" of "_UFunc_Nin1_Nout1" has incompatible type "Any | None"; expected "complex | str | bytes | generic[Any]" [arg-type]
pyorbital/orbital.py:464: error: Value of type variable "SupportsRichComparisonT" of "max" cannot be "signedinteger[_32Bit | _64Bit] | Any | None" [type-var]

now back to 5

(.venv) giorgio@giorgio-Aspire-V3-572G:~/pyorbital$ mypy pyorbital
pyorbital/orbital.py:42: error: Incompatible types in assignment (expression has type "None", variable has type Module)  [assignment]
pyorbital/orbital.py:49: error: Incompatible types in assignment (expression has type "None", variable has type Module)  [assignment]
pyorbital/tests/test_astronomy.py:38: error: Cannot assign to a type  [misc]
pyorbital/tests/test_astronomy.py:38: error: Incompatible types in assignment (expression has type "None", variable has type "type[DataArray]")  [assignment]
pyorbital/geoloc_example.py:29: error: Skipping analyzing "mpl_toolkits.basemap": module is installed, but missing library stubs or py.typed marker  [import-untyped]
pyorbital/geoloc_example.py:29: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 5 errors in 3 files (checked 20 source files)

[djhoese]

Ok those errors are from ImportError hackery. I'm OK ignore them for now and in the future we can add an inline type ignore comment.

@mraspaud @pnuu do either of you have time to run Satpy's unit tests against this PR and make sure it passes so we're not surprised later?

[pnuu]

I'll have a go running the Satpy tests.

[mraspaud]

I also have tests I can run against other libs

[djhoese]

Unrelated: @JaskRendix this project is planned to be relicensed from GPL to Apache version 2. Once we organize some of the other dependencies we'll ask all past contributor's if they're OK with this in a GitHub issue. This is just a heads up.

[pnuu]

With Satpy I only get an unrelated failure in downloading TLEs from Celestrak, most likely our network is being throttled or something. So all good from Satpy's perspective!

[pnuu]

Pytroll-collectors and Trollflow2 are fine, but I got a failure in Pytroll-Schedule in https://github.com/pytroll/pytroll-schedule/blob/main/trollsched/tests/test_schedule.py#L221 , the expected rise time and resulting value differ by few milliseconds. Might be close enough :sweat_smile: This is most likely coming from the bug you fixed in the previous PR. I'll fix this in Pytroll-Schedule, we might not need sub-second accuracy in scheduling...

rt1 = datetime.datetime(2018, 11, 28, 10, 53, 42, 79483)
rise_times = [datetime.datetime(2018, 11, 28, 10, 53, 42, 66585),
                       datetime.datetime(2018, 11, 28, 12, 34, 44, 662042)]

[djhoese]

Ref: https://github.com/pytroll/pytroll-schedule/pull/110

[djhoese]

Now that pytroll-schedule is cleaned up, do we have any other issues with this PR? Merge?

[mraspaud]

I haven't tested it yet, I'll try to get to it tonight

[djhoese]

Ah sorry, I misread your previous comment to mean you had tested those things.