search

coralnet / pyspacer

Python based tools for spatial image analysis
MIT License
6 stars 2 forks source link

Fix bucket_name bucketname consistency #88

Closed yeelauren closed 5 months ago

yeelauren commented 5 months ago

Separating out into smaller PRs :smile_cat:

Refactor for consistency see: https://github.com/coralnet/pyspacer/blob/cb0b427b03f60bb92e2e9bdf1ccbb64e5b2230c3/spacer/storage.py#L187

https://github.com/coralnet/pyspacer/blob/cb0b427b03f60bb92e2e9bdf1ccbb64e5b2230c3/spacer/storage.py#L205

https://github.com/coralnet/pyspacer/blob/cb0b427b03f60bb92e2e9bdf1ccbb64e5b2230c3/spacer/messages.py#L25

yeelauren commented 5 months ago

Future update on a new change from #85 will also need to be changed https://github.com/coralnet/pyspacer/blob/478a8f6f01d7bdc1cb7b40866d36d89b53dda314/spacer/messages.py#L71

StephenChan commented 5 months ago

Thanks Lauren! This'll make reviewing easier.

I'm definitely onboard with making this param name consistent, but:

  1. It looks like there are still a decent amount of bucket_name usages remaining: for example in messages.py, storage.py, tests/test_storage.py, tests/utils.py, and README.md.
  2. Pardon my pickiness, but unless there's another reason I've overlooked (e.g. consistency with boto), I think I'd actually favor bucket_name over bucketname. The style goes better with the storage_type param in DataLocation.__init__() and storage_factory(); and originally there were more usages of bucket_name than bucketname (around 50 vs. 10).
yeelauren commented 5 months ago

@StephenChan I agree that makes more sense - the latest push should address those changes !