Refactor and improve tests concerning the TLE file handling #118

adybbroe · 2023-02-15T09:52:47Z

This PR was originally started to introduce support for NOAA-21.

The PR started out attempting to address an issue found when the user requests TLEs for a name not matching what is in the platforms.txt file and only partially matching what name is written in the TLE files. In particular this was a bit confusing when NOAA-21 was newly introduced and the name was NOAA 21 (JPSS-2)in the TLE files from Space_Track/Celetrak, while for NOAA-20 the name was NOAA 20 in the same files.

Support for NOAA-21 was actually already included in #113 but that PR is about environment variables.

Closes tlefile.py seem to use satellite name rather than the international designator number #119
Tests added
Fully documented

…txt file Signed-off-by: Adam.Dybbroe <[email protected]>

…e automated bots for checking code quality and tests Signed-off-by: Adam.Dybbroe <[email protected]>

codecov · 2023-02-15T09:58:21Z

Codecov Report

Attention: Patch coverage is 93.18182% with 45 lines in your changes missing coverage. Please review.

Project coverage is 88.36%. Comparing base (8d982a9) to head (3499641).
Report is 124 commits behind head on main.

Files with missing lines	Patch %	Lines
pyorbital/orbital.py	86.00%	14 Missing ⚠️
pyorbital/fetch_tles.py	0.00%	9 Missing ⚠️
pyorbital/tlefile.py	91.66%	7 Missing ⚠️
pyorbital/geoloc_example.py	0.00%	6 Missing ⚠️
pyorbital/astronomy.py	88.57%	4 Missing ⚠️
pyorbital/tests/test_astronomy.py	92.50%	3 Missing ⚠️
pyorbital/check_platform.py	0.00%	1 Missing ⚠️
pyorbital/tests/test_tlefile.py	99.60%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #118      +/-   ##
==========================================
+ Coverage   87.51%   88.36%   +0.85%     
==========================================
  Files          15       16       +1     
  Lines        2211     2373     +162     
==========================================
+ Hits         1935     2097     +162     
  Misses        276      276

Flag	Coverage Δ
unittests	`88.36% <93.18%> (+0.85%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

adybbroe · 2023-02-15T12:41:33Z

While working with this PR and upgrading our systems for NOAA-21 at SMHI we realized that there seem to be a bug here, standing in the way of updating support for NOAA-21, see #119
So, perhaps this PR should try fix this as well?

Signed-off-by: Adam.Dybbroe <[email protected]>

# Conflicts: # pyorbital/tests/test_tlefile.py # pyorbital/tlefile.py

pyorbital/tests/test_tlefile.py

coveralls · 2024-11-23T11:41:12Z

coverage: 88.326% (+0.4%) from 87.904%
when pulling 3499641 on adybbroe:add-jpss2
into 9700076 on pytroll:main.

adybbroe · 2024-11-23T13:43:25Z

I have come to the conclusion that there is really no serious issue easily solved as otherwise suggested in #119
Either the user specify a WMO Oscar based satellite name matching what is in the platforms.txt, so NOAA-21 (already in main) or the user must specify an exact match to what is written in the TLE files. So if the TLE files look like this:

NOAA 21 (JPSS-2)        
1 54234U 22150A   23045.56664999  .00000332  00000+0  17829-3 0  9993
2 54234  98.7059 345.5113 0001226  81.6523 278.4792 14.19543871 13653

The use must use either WMO-Oscar satellite name NOAA-21or specify NOAA 21 (JPSS-2) as the platform name, as in:
tle_jpss2 = tlefile.read('NOAA 21 (JPSS-2)', './jpss2.tle')

The only thing this PR does now is to add a debug message indicating that there might be a match in the TLE file for the the satellite the user is wanting the TLEs for. So, like:

>>> tle_metop = tlefile.read('Metop', '/data/tles/tle-202409060015.txt')
[DEBUG: 2024-11-23 14:30:03 : pyorbital.tlefile] Path to the Pyorbital configuration (where e.g. platforms.txt is found): /home/a000680/usr/src/forks/pyorbital/pyorbital/etc
[DEBUG: 2024-11-23 14:30:03 : pyorbital.tlefile] Found a possible match: METOP-B?
[DEBUG: 2024-11-23 14:30:03 : pyorbital.tlefile] Found a possible match: METOP-C?

And:

>>> tle_n21 = tlefile.read('NOAA 21', './jpss2.tle')
[DEBUG: 2024-11-23 14:38:15 : pyorbital.tlefile] Path to the Pyorbital configuration (where e.g. platforms.txt is found): /home/a000680/usr/src/forks/pyorbital/pyorbital/etc
[DEBUG: 2024-11-23 14:38:15 : pyorbital.tlefile] Found a possible match: NOAA 21 (JPSS-2)?

It means that in case the user gives a name like "NOAA" or "Metop" or even "N" or something far from a real satellite name, there might be several debug messages with more or less useful suggestions. But I think that can be accepted.

Okay?

Signed-off-by: Adam.Dybbroe <[email protected]>

pnuu · 2024-11-25T13:39:22Z

pyorbital/tlefile.py

+    elif l_0.startswith(platform) and platform not in SATELLITES:
+        LOGGER.debug("Found a possible match: %s?", str(l_0.strip()))


I'm wondering should there be something other than just the log message here? The tests pass, but 🤷‍♂️

I'm wondering should there be something other than just the log message here? The tests pass, but 🤷‍♂️

Did you read my discussion above?

Nope, just wanted to solve the pre-commit complaint and afterwards got confused about the former title and description of the issue 😅

Should the message say that the possible match isn't used? Something like

Suggested change

elif l_0.startswith(platform) and platform not in SATELLITES:

LOGGER.debug("Found a possible match: %s?", str(l_0.strip()))

elif l_0.startswith(platform) and platform not in SATELLITES:

LOGGER.debug("Found a partial match: %s. TLE not used.", str(l_0.strip()))

pnuu

Umm, this PR currently actually does only do refactoring and not add the mentioned NOAA-21 to the platforms.txt. So git commit missing @adybbroe ?

adybbroe · 2024-11-25T13:44:43Z

Umm, this PR currently actually does only do refactoring and not add the mentioned NOAA-21 to the platforms.txt. So git commit missing @adybbroe ?

Yeah right, after a long discussion (mainly with myself) it ended up differently from what I anticipated when I started. Should we rename it then? I am okay with that!

adybbroe · 2024-11-25T15:12:57Z

Umm, this PR currently actually does only do refactoring and not add the mentioned NOAA-21 to the platforms.txt. So git commit missing @adybbroe ?

Better now @pnuu ?

pnuu · 2024-11-26T06:45:57Z

Better now @pnuu ?

Much better! Now the title and description match what's being changed 👍

Add NOAA-21 to the list of designators/ids in the built in platforms.…

bfe39f6

…txt file Signed-off-by: Adam.Dybbroe <[email protected]>

adybbroe self-assigned this Feb 15, 2023

adybbroe requested a review from mraspaud February 15, 2023 09:52

adybbroe added the enhancement label Feb 15, 2023

Fix PR template to reflect actual main branch and the fact that we us…

902e290

…e automated bots for checking code quality and tests Signed-off-by: Adam.Dybbroe <[email protected]>

Add test cases and add draft solution (commented out atm) for discussion

6f42f15

Signed-off-by: Adam.Dybbroe <[email protected]>

adybbroe mentioned this pull request Feb 17, 2023

tlefile.py seem to use satellite name rather than the international designator number #119

Open

Merge branch 'main' into add-jpss2 and fix unittest

e828412

# Conflicts: # pyorbital/tests/test_tlefile.py # pyorbital/tlefile.py

adybbroe requested review from gerritholl and pnuu November 20, 2024 15:36

Merge branch 'main' into add-jpss2

cf3f268

adybbroe commented Nov 23, 2024

View reviewed changes

pyorbital/tests/test_tlefile.py Outdated Show resolved Hide resolved

adybbroe commented Nov 23, 2024

View reviewed changes

pyorbital/tests/test_tlefile.py Outdated Show resolved Hide resolved

adybbroe commented Nov 23, 2024

View reviewed changes

pyorbital/tests/test_tlefile.py Outdated Show resolved Hide resolved

adybbroe commented Nov 23, 2024

View reviewed changes

pyorbital/tests/test_tlefile.py Outdated Show resolved Hide resolved

adybbroe added 3 commits November 23, 2024 12:31

Update pyorbital/tests/test_tlefile.py

4ae66b1

Update pyorbital/tests/test_tlefile.py

e8d8b21

Update pyorbital/tests/test_tlefile.py

54ccf2d

Adam.Dybbroe and others added 7 commits November 23, 2024 14:55

Add debug message to try help the user entering the right platform name

1441016

Signed-off-by: Adam.Dybbroe <[email protected]>

Merge branch 'add-jpss2' of github.com:adybbroe/pyorbital into add-jpss2

121a08b

Fix sytle issues and make ruff happier

007534e

Signed-off-by: Adam.Dybbroe <[email protected]>

Style fix: PT006

a8db385

Signed-off-by: Adam.Dybbroe <[email protected]>

Refactor the function to get TLEs from URIs

7552a16

Signed-off-by: Adam.Dybbroe <[email protected]>

Move imports from pyorbital to test functions

6c977cd

Refactor _get_tles_from_uris

3499641

pnuu reviewed Nov 25, 2024

View reviewed changes

pnuu requested changes Nov 25, 2024

View reviewed changes

adybbroe changed the title ~~Add NOAA-21 support~~ Refactor and improve tests concerning the TLE file handling Nov 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor and improve tests concerning the TLE file handling #118

Refactor and improve tests concerning the TLE file handling #118

adybbroe commented Feb 15, 2023 •

edited

Loading

codecov bot commented Feb 15, 2023 •

edited

Loading

adybbroe commented Feb 15, 2023

coveralls commented Nov 23, 2024 •

edited

Loading

adybbroe commented Nov 23, 2024 •

edited

Loading

pnuu Nov 25, 2024

adybbroe Nov 25, 2024

pnuu Nov 26, 2024

pnuu Nov 26, 2024 •

edited

Loading

pnuu left a comment

adybbroe commented Nov 25, 2024

adybbroe commented Nov 25, 2024

pnuu commented Nov 26, 2024

		elif l_0.startswith(platform) and platform not in SATELLITES:
		LOGGER.debug("Found a possible match: %s?", str(l_0.strip()))

Refactor and improve tests concerning the TLE file handling #118

Are you sure you want to change the base?

Refactor and improve tests concerning the TLE file handling #118

Conversation

adybbroe commented Feb 15, 2023 • edited Loading

codecov bot commented Feb 15, 2023 • edited Loading

Codecov Report

adybbroe commented Feb 15, 2023

coveralls commented Nov 23, 2024 • edited Loading

adybbroe commented Nov 23, 2024 • edited Loading

pnuu Nov 25, 2024

Choose a reason for hiding this comment

adybbroe Nov 25, 2024

Choose a reason for hiding this comment

pnuu Nov 26, 2024

Choose a reason for hiding this comment

pnuu Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

pnuu left a comment

Choose a reason for hiding this comment

adybbroe commented Nov 25, 2024

adybbroe commented Nov 25, 2024

pnuu commented Nov 26, 2024

adybbroe commented Feb 15, 2023 •

edited

Loading

codecov bot commented Feb 15, 2023 •

edited

Loading

coveralls commented Nov 23, 2024 •

edited

Loading

adybbroe commented Nov 23, 2024 •

edited

Loading

pnuu Nov 26, 2024 •

edited

Loading