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

Redesign API to replace ad-hoc type hints in param names with actual type hints #323

Open
phargogh opened this issue May 31, 2023 · 1 comment
Assignees
Labels
task Changes that do not affect code functionality
Milestone

Comments

@phargogh
Copy link
Member

phargogh commented May 31, 2023

We have a variety of parameters throughout pygeoprocessing where the parameter name includes the type of the parameter. Examples include:

  • base_raster_path_band_const_list in raster_calculator
  • base_raster_path_listin align_and_resize_raster_stack
  • resample_method_list in align_and_resize_raster_stack
  • raster_driver_creation_tuple in a number of functions

I don't remember why, exactly, we choose to denote type this way and not to use type hints, but type hints these days are pretty well-supported and a descriptive way for us to render the input types in places like the API docs.

If we do this, we should be careful about the update, since this will change a large number of parameter names throughout the package and break everything that calls it in the process.

So the proposal here is to do a major API change sometime in the future (pygeoprocessing 3.0 or 4.0) so that

  • resample_method_list becomes resample_methods
  • base_raster_path_band_const_list becomes something simpler, maybe inputs?

A trimmed down version of this that wouldn't break compatibility would simply be to add type hints everywhere, which itself would be a nice addition.

@phargogh phargogh added the proposal Proposals requiring team feedback label May 31, 2023
@phargogh phargogh self-assigned this May 31, 2023
@phargogh
Copy link
Member Author

phargogh commented Jun 1, 2023

We talked about this on the team call this morning and agreed that while a design doc is not necessary, it would be helpful to have a shared doc to review the changes and discuss. Many parameter names that could benefit from the change are shared across functions, so this would be a consolidated list.

We also talked about how this would make sense to go in a major pygeoprocessing release, perhaps along with a few other things like supporting URLs and formally handling the forthcoming GDAL 4.0 change where exceptions are enabled by default. Changes like these may end up on a roadmap of some kind through github projects.

@phargogh phargogh added task Changes that do not affect code functionality and removed proposal Proposals requiring team feedback labels Jun 1, 2023
@phargogh phargogh changed the title Proposal: redesign API to replace ad-hoc type hints in param names with actual type hints Redesign API to replace ad-hoc type hints in param names with actual type hints Jun 1, 2023
@phargogh phargogh added this to the 3.0 milestone Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task Changes that do not affect code functionality
Projects
None yet
Development

No branches or pull requests

1 participant