Open atrawog opened 4 years ago
Wow, thanks a lot! That is actually the first external pull request 😃.
I am puzzled that you managed to find such a simple solution. By coincidence, I was fixing this issue today (see #31) as part of a complete overhaul of the satellite subpackage. I will look into your solution, make sure that it runs with both, v1 and v2 of sgp4 and accept your PR, as it is much simpler. Perhaps, I will apply some of my changes on top of it.
That said, as part of the overhaul, I'm going to deprecate the current interface. When I worked on the cysgp4 package, I came up with a much cleaner solution, which is also much faster. As was mentioned in #30, it would be good to have the choice to also use python-sgp4 for that. I think, it should be possible to make a similar routine such as the propagate_many based on it. (In fact, there is already propagate_many_sgp4, but it lacks some functionality, which I'm going to add.)
It is planned also, to add functions and tutorial notebooks for so-called EPFD simulations, that are used in spectrum management to assess the aggregate (summed-up) received power of a constellation of satellites. If you are interested in that, check out our contributions to CEPT ECC/SE40 group, in particular the software. @fdivruno and I will merge this into pycraf
soon.
Feel free to propose other features or contribute.
In fact, as I cannot easily make changes to your PR, can you try, what happens if you change the line "sgp4<2" to "sgp4>2" in azure-pipelines.yml:205
? That would make sure, that the bugfix works on all platforms. I have the suspicion that the new version might break the examples in the documentation - at least that happened to me, as the .epoch
attribute was missing in the new sgp4 version. But that would be easy to fix.
I looked into this in a little bit more detail. When you look at the python-sgp4
documentation on PyPI, you'll see that there example imports the relevant classes from sgp4.api
. I was now wondering, why it would work to use the Classes from sgp4.model
(or previously sgp4.io
). I think the reason is that sgp4.api
is a high-level interface, which determines whether the compiled C++ wrapper should be used or rather the legacy Python-implementation. If I'm not mistaken, importing from sgp4.model
directly uses the slow Python-only implementation. I guess, for now this is OK, given the plan to deprecate this code path anyway.
python-sgp4 is a somewhat moving target at the moment and the changes made to sgp4.io and sgp4.model in version 2.11 seem to breaks backward compatibility to 1.4 https://github.com/brandon-rhodes/python-sgp4/commit/ad0853263d467296479b545482bafde8fe466135#diff-fb82ffdfc8b747ffe75318adb2b5d613
To make things even more complicated Conda is currently providing sgp4 version 2.10 while PyPi is at version 2.12. So a dependency sgp4>2 (currently) works fine in Conda, but breaks things for everyone else.
So it's probable time to write a small abstraction module for pycraf to deal with all the small differences between SGP4 implementations or only use cysgp4 for everything in future.
This fixes both the the failed import of the Satellite Object in the current sgp4 version 2.12 and make pycraft raise less confusing error exceptions (at the moment pycraft reports the sgp4 module missing when the sgp4 import works fine, but a submodule is missing).
This fix should be backward compatible with previous version of sgp4, but it's untested.