search

pytroll / pyorbital

Orbital and astronomy computations in python
http://pyorbital.readthedocs.org/
GNU General Public License v3.0
224 stars 77 forks source link

add get all platforms function and generator getting one platform to processing #84

Open 19as opened 2 years ago

19as commented 2 years ago
19as commented 2 years ago

please, merge my request in trunk

djhoese commented 2 years ago

Oh also could you try adding some tests for these functions?

djhoese commented 2 years ago

@19as Any updates on this? I had some questions and comments that I was hoping you could answer before we move forward with this pull request. If you have questions for me let me know. If you don't reply in the next week or two then I will plan on closing this.

19as commented 2 years ago

Sorry about the delay, we've been kind of busy and sometimes pull requests get lost in the inbox. How do you plan on using this functionality?

I know very little about TLE files so correct me if I'm missing something, but it looks like your two functions are very similar and possibly don't do what they say. The top one says that it gets multiple platforms and the bottom one says it gets a single one. However, the second one is a generator so it actually yields all the platforms too.

Lastly, these functions are almost exactly the same code. Could they be combined? Or one could use the other? If the only difference between these is a list versus a generator then I would suggest having only the generator and leave it up to the user to call list(get_platform(tle_file)) instead of doing it for them.

You're right. It is better to use one generator. I had to go through the file and, upon receiving one aircraft, process it

19as commented 2 years ago

Oh also could you try adding some tests for these functions?

I will try to do it in the future