Sprutner/add seatmap explorer

[sprutner]

This adds functionality to the amadeus python SDK to allow
use of the "SeatMap Display" API endpoint within the
python SDK.

Changes for this pull request

Add seatmap explorer functionality

[tsolakoua]

Thank you so much for your PR @sprutner ! We reviewed your commits and tests are missing as according to our contributor's guide every contributor has to make sure new tests are included when a functionality is implemented.

[sprutner]

Sure, I can do that. One thing I am struggling with is I need a good example flight-orderId. I was using an offer-id which in the docs says should work but when I talked to amadeus support they told me this is not possible. I'm not sure what order-id to use because I haven't ordered any flights. I will need to update the docstring example to reflect this.

Looking at previous commits, it looks like you have posted {"foo": "bar"} or (a="b") as mock data. Should I just go ahead and use those for my tests a well?

[tsolakoua]

One thing I am struggling with is I need a good example flight-orderId. I was using an offer-id which in the docs says should work but when I talked to amadeus support they told me this is not possible. I'm not sure what order-id to use because I haven't ordered any flights. I will need to update the docstring example to reflect this.

In order to take a valid order-id you need to call the Flight Create Order API, as the order-id is not permanent. When a booking is created in test environment using this API, we keep this booking temporary so users can still try to do bookings.

Looking at previous commits, it looks like you have posted {"foo": "bar"} or (a="b") as mock data. Should I just go ahead and use those for my tests a well?

Yes, you can use the same mock data.

Hope that helps! Let us know if you have further questions or doubts!

[tsolakoua]

Thank you for updating the tests and docs @sprutner! We reviewed your commits and there is a small part missing from the tests:
In the namespaces_spec.py we define the paths and expected .get() methods, for example in the case of Trip Purpose Prediction we have:
expect(client.travel.predictions.trip_purpose).not_to(be_none) (for the expected path)
expect(client.travel.predictions.trip_purpose.get).not_to(be_none) (for the .get() method).

Thanks again and let us know if you have any questions :)

[sprutner]

Seem to be getting a fail with the travis build on py3.6.3

[tsolakoua]

Seem to be getting a fail with the travis build on py3.6.3

We'll check it out ASAP.

[tsolakoua]

We did an update on master and rebased your branch. The problem was the environmental variables set in Travis for our amadeus/python repo seem not to be available when a PR is submitted from a separate forked repository like your case. It should be fine now with the changes we introduced. Just before you merge your PR please check out the duplications we identified in the last commits below:

• seatmap example in readme appears twice, first and second
• seatmap post tests appears twice first and second

Also we have a question; why do you use the double ** asterisk when passing the parameters in the get method, for example amadeus.shopping.seatmaps.get(**{"flight-orderId": "orderid"})? It works fine but it is something we haven't used before in our examples and we wanted to know what value it brings calling the method that way.

Thanks!

[sprutner]

Hey @tsolakoua I will go take a look at making those deduplications. Regarding the double **, I believe this was from an example I copied. This was my first time trying to set an arg that contains a hyphen within the arg name, so flight-orderId does not work as python picks that up as a subtraction operator. So I found an alternative way to pass that argument with a hyphen in. If it works without a ** I will just remove that for consistency!

[sprutner]

Okay so it looks like the double ** is required in this case. I also updated the tests to use the ** way of passing in the args, and got rid of my duplicate tests and the readme entries.

I noticed you also have a couple duplicates for Flight Offer Search here as well in the readme
first
second

I will leave that change out of this PR but you could remove it in a future PR.

Thank you for the feedback! Please re-review and let me know if we are good to go!

[tsolakoua]

Indeed without the ** it's not working! The duplications in the master branch don't exist in the readme file, but it's fine we can take care of them afterwards. We are good to go :) Thank you @sprutner !

[sprutner]

Sounds good. Looks like I don't have permissions to merge it so go ahead whenever you are ready for it.

[tsolakoua]

@sprutner Merged! Thank you so much for your contribution! 🎉