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

Generate config files + run download in loop #33

Merged
merged 5 commits into from
Sep 2, 2022

Conversation

crangelsmith
Copy link
Collaborator

Fixes #31

@crangelsmith crangelsmith marked this pull request as ready for review August 30, 2022 12:52
Copy link
Collaborator

@andrewphilipsmith andrewphilipsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good to me.

There is one change that I think should be made (assuming I've understood it correctly). In generate_config_file.py, I think the bounds file represents the GEE image downloads, not the chips for inference - Is that right? If so, then the variable name chips_gdf is misleading. I've changed this to bounds_gdf. I've also tweaked the CLI parameter description to remove the word "chips".

I've added a small commit with these changes. Feel free to revert my change if I've got it wrong.

@crangelsmith
Copy link
Collaborator Author

Generally looks good to me.

There is one change that I think should be made (assuming I've understood it correctly). In generate_config_file.py, I think the bounds file represents the GEE image downloads, not the chips for inference - Is that right? If so, then the variable name chips_gdf is misleading. I've changed this to bounds_gdf. I've also tweaked the CLI parameter description to remove the word "chips".

I've added a small commit with these changes. Feel free to revert my change if I've got it wrong.

Thank you @andrewphilipsmith, this makes sense! I'll fix the broken test and merge it.

@crangelsmith crangelsmith merged commit 4c9dc46 into main Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt config files to accept geodataframe/parquet file, in place of bounds
2 participants