-
Notifications
You must be signed in to change notification settings - Fork 6
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
Adding in the Tiles class. #8
Adding in the Tiles class. #8
Conversation
Hello, I'm sorry for my delay in following up on this. I unfortunately did not have the time or energy to address this when you originally submitted it. But, I'm ready to try to get this merged in now as its a feature that should be included in this library. I understand if you have moved onto other things in the time since you created this pull request, so I'm happy to take this over if you are no longer interested. There are a few things I would like changed before we merge this in, so I'll wait a week to hear back from you before moving forward. |
Hey man!
Yeah I was mainly interested in using this library at my previous job for a DTED related use case, but I would be more than happy to make any requested edits and submit another pull request! Your implementation of DTED reading was BY FAR the most user friendly API of the different python libraries out there (and clean code)
not sure how the workflow usually goes for open source software but I'm up to collaborate :)
let me know on next steps
On 2/25/2023 7:42:05 PM, Ben Bonenfant ***@***.***> wrote:
Hello, I'm sorry for my delay in following up on this. I unfortunately did not have the time or energy to address this when you originally submitted it. But, I'm ready to try to get this merged in now as its a feature that should be included in this library.
I understand if you have moved onto other things in the time since you created this pull request, so I'm happy to take this over if you are no longer interested. There are a few things I would like changed before we merge this in, so I'll wait a week to hear back from you before moving forward.
—
Reply to this email directly, view it on GitHub [#8 (comment)], or unsubscribe [https://github.com/notifications/unsubscribe-auth/ARCWKTRNRXSCXTRKAF7YDM3WZKYG3ANCNFSM56CNR72Q].
You are receiving this because you authored the thread.Message ID: ***@***.***>
[4c53e1e3-94e3-4a16-9ece-de1913273723]
|
Thank you for the kind words, I'm just happy to have made a tool people want to use. I'll add some review comments for the things I'd like to see changed. You can either address them or I can, since by default I have permission to edit this PR. So, for example, one thing this code change needs is tests, but I can write those for you so you don't need to learn how the tests work. I'm going to test out the new |
Okay yeah that sounds great. Please test it out and see if it works appropriately! You are free to make any edits to the PR if it would be faster than explaining what u would like changed :) |
…arsing DTED files (bbonenfant#9) * Add warn kwarg to Tile.__init__ and Tile.load_data methods. --------- Co-authored-by: Ben Bonenfant <[email protected]>
Hey there, This library is fantastic and exactly what I am looking for for a job. Especially if this Tilesets PR is complete. Is there more work to be done here? How can I help? |
Apologies, this really slipped by me. I think I originally wanted to include functionality to query ZIP files, and then it fell to the wayside. But this functionality really shouldn't be blocked on that. This PR is so far behind, and old, that I don't want to potentially mess with Weston's branch. So I made a new PR with these changes that should be ready to merge in. @BenShoeSpectric take a look at #13 if you have the time and I'll merge it in tomorrow and cut a new release. TLDR: Closing this for #13 |
Thanks for getting this merged in on the other PR! I'm glad the concept made it in |
New Tiles class added.