agentmorris / MegaDetector

MegaDetector is an AI model that helps conservation folks spend less time doing boring things with camera trap images.
MIT License
103 stars 24 forks source link

Update get_lila_annotation_counts.py script to work with metatdata schema #124

Closed nathanielrindlaub closed 5 months ago

nathanielrindlaub commented 5 months ago

Hey @agentmorris,

I was just poking around your LILA scripts and noticed a small bug in this line of the get_lila_annotation_counts.py script.

It looks like the image_base_url key no longer exists in the dict returned by lila_read_metadata(), and instead the image base url headers now follow this pattern: image_base_url_<cloud_provider>.

I fixed this by adding preffered_cloud = 'gcp' to the top of the file and modifying that line to:

    base_url = metadata_table[ds_name]['image_base_url_' + preferred_cloud]

Happy to open a PR for that change if you'd like.

agentmorris commented 5 months ago

Wow, you're fast! That change to the metadata just happened this weekend. Yes, that's the same change I would have made, please open a PR. Thanks!

nathanielrindlaub commented 5 months ago

@agentmorris no problem at all! It looks like I need write access to push a branch/open a PR?

agentmorris commented 5 months ago

I'm happy to give you write access because I know you :), but... I think standard practice is for you to fork the repo, make your changes there, then create a PR from your repo into this repo. E.g. this recent PR was submitted by a contributor who does not have write access to the repo, and that went smoothly.

GitHub's documentation on this process is clear and helpful, except for the confusing part where it says that you need write access; I'm 99.999% sure they mean that you need write access to the source repo, which in this case would be your fork.

Try that and let me know if you hit any snags? Thanks!

nathanielrindlaub commented 5 months ago

Ah! I rarely contribute to external repos so had no idea you could create a PR from a forked project. Thanks I think that got me sorted.

agentmorris commented 5 months ago

Merged, closing this issue. Thanks for finding this!