Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Closest TAF #15

Closed
speshak opened this issue Nov 16, 2019 · 13 comments
Closed

Add Closest TAF #15

speshak opened this issue Nov 16, 2019 · 13 comments
Labels
enhancement New feature or request

Comments

@speshak
Copy link
Contributor

speshak commented Nov 16, 2019

When you look up the weather for an airport that doesn't have a TAF, the TAF section is understandably blank. It would be nice for it to instead display the nearest TAF. Another option would be a way to select from several "nearby" TAFs and the user can determine which is most applicable to their use.

@AJWurts AJWurts added the enhancement New feature or request label Nov 17, 2019
@speshak
Copy link
Contributor Author

speshak commented Nov 18, 2019

It looks like NWS's ADDS service has an endpoint that might be useful for this: https://aviationweather.gov/dataserver/example?datatype=taf#radialdist

@AJWurts
Copy link
Owner

AJWurts commented Nov 18, 2019

That looks like it will work. I will try to add it later this week.

@speshak
Copy link
Contributor Author

speshak commented Nov 18, 2019

I have some capacity to take a whack at it myself too if you'd prefer.

@AJWurts
Copy link
Owner

AJWurts commented Nov 18, 2019

The backend shouldn't be too difficult. Give it a shot. My idea would be to add a new API endpoint. It takes a radial distance, latitude, longitude, and limit. It would call the NWS api for TAFs, process each and return them. The frontend will be modified to use this endpoint instead of the current one.

@speshak
Copy link
Contributor Author

speshak commented Nov 18, 2019

That's pretty close to what I had envisioned too, though I was going to have the endpoint take an airport code instead of a lat/long. I planned on using the same API you're already using to look up runway information to get the airport coordinates. That felt more in line with how the other endpoints work, and keeps the client from needing to look up the airport coordinates.

@AJWurts
Copy link
Owner

AJWurts commented Nov 19, 2019

Yea that would work better for TAFs. I was thinking about providing the nearest airports for searching, which is the other issue.

@speshak
Copy link
Contributor Author

speshak commented Nov 19, 2019

I got a good POC put together: https://github.com/speshak/WeatherVis/tree/nearby_taf
How committed are you to the TAF JSON structure? My code does a pretty dumb conversion from the NWS XML to JSON, and it might be easier to update the client side to support that data format. (My branch has a commit that also updates the newestTAFS endpoint to use the same format via the addsclient class that I added)

@AJWurts
Copy link
Owner

AJWurts commented Nov 19, 2019

Parsing the XML is a better strategy and less prone to errors. If we converted the XML to JSON then matched up the keys with the current json format that would be the least amount of work. I can do that today. It should dramatically simplify the project.

@speshak
Copy link
Contributor Author

speshak commented Nov 19, 2019

Sounds good. I think the gnarliest part of this new format is that the xml2js module I used ends up wrapping each data value in an array. I think I'll add a filter step that unpacks those to make for some saner objects.. Nevermind, turns out there there is an option to turn this behavior off.

I've also got an additional commit that switches over the METAR fetching to the ADDS XML as well, but it looks like I didn't push it. I'll get that pushed tonight.

@AJWurts
Copy link
Owner

AJWurts commented Nov 19, 2019

I added a conversion from XML json to the original JSON. It is in changeToXML. I also changed cloud data to be stored in a list instead of individual values.

@speshak
Copy link
Contributor Author

speshak commented Nov 20, 2019

This issue has seen a wee bit of scope creep. 😕
I merged our two branches and opened a PR (#27). This PR isn't quite ready to merge because it has my change to use ADDS for METAR without the corresponding change to the frontend.

@AJWurts
Copy link
Owner

AJWurts commented Nov 21, 2019

I think we could improve this by showing the nearest 5 (or more) TAF reports for every airport. Above the TAF section could be a set of buttons corresponding to the airports that the user could click through. Was this what you were thinking? Or was this only for airports without TAFs? Also I added a Slack link to the readme, so we could chat without polluting issue discussions.

@AJWurts
Copy link
Owner

AJWurts commented Nov 29, 2019

#33 Finished issue.

@AJWurts AJWurts closed this as completed Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants