xoolive / traffic

A toolbox for processing and analysing air traffic data
https://traffic-viz.github.io/
MIT License
371 stars 79 forks source link

Support threshold coordinate projection via compute_xy() interface #326

Open sfo opened 1 year ago

sfo commented 1 year ago

This PR intends to add support for projecting threshold coordinates using the common compute_xy() interface.

However, currently the RunwayAirport() constructor just swallows the enriched DataFrame passed to it at the end of compute_xy(). Consequently, the following code returns None:

>>> from traffic.data import airports
>>> airport = airports["CDG"]
>>> airport.runways.compute_xy()
None

The DataFrame containing the projection can be accessed via

>>> airport.runways.compute_xy()._data
    latitude  longitude     bearing name            x            y
0  48.995701    2.55274   85.293092  08L  -722.812433 -1562.345186
1  48.998798    2.61018  265.336441  26R  3480.306639 -1216.826055
2  48.992901    2.56566   85.263542  08R   222.653855 -1873.773350
3  48.994900    2.60243  265.291290  26L  2913.454043 -1650.716705
4  49.024700    2.52489   85.264343  09L -2759.230258  1663.306856
5  49.026699    2.56169  265.292128  27R   -67.826773  1884.919854
6  49.020599    2.51306   85.267941  09R -3624.711297  1207.753756
7  49.023701    2.57029  265.311147  27L   561.147681  1551.500392

I am currently not sure how to propagate the results of the projection back up the runway/threshold data stream.

xoolive commented 1 year ago

Thank you for the update.

Indeed, that structure is not great now 😢 It would need a serious refactoring that has not been done yet (and it's too late for tonight 😇 to start that!)

It's not very natural to model the whole list of runways with a dataframe because the thresholds come by two (and you need the two thresholds to plot the whole runway for instance)

The .data attribute is not an attribute but a property, which computes a view on which you use .compute_xy() to get the x,y parameters, build a new dataframe but the proper way to instantiate a RunwayAirport class should be from a list of pairs of thresholds.

This is obviously not great, but I never took time to think about how to improve it.

Honestly I don't know... So I am not really enthusiast about merging this as is, because even if "of course it wouldn't really hurt", it would be just a "band-aid on a wooden leg" (French colloquialism 🤷)

I would be more than happy to convert that into an issue and think about how to improve that part of the library though!

What do you think?