Skip to content

Conversation

@valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Feb 4, 2021

Description

@mwjury shied away from using the utilities library as described by himself in this comment because he found the bounds fixing problematic for his particular use case. So I introduced a set of user args that throttle bounds fixing (or not). Note that when bounds are None they will always be fixed regardless of user input!

  • Closes #issue_number
  • Link to documentation:

Before you get started

Checklist

It is the responsibility of the author to make sure the PR is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

New or updated recipe/diagnostic:

New or updated data reformatting script:


To help with the number pull requests:

@valeriupredoi
Copy link
Contributor Author

can I haz a technical reviewer here please mateys @schlunma @jvegasbsc @bouweandela

@jvegreg
Copy link
Contributor

jvegreg commented Feb 16, 2021

I'll do

Copy link
Contributor

@jvegreg jvegreg left a comment

Choose a reason for hiding this comment

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

I do not like that the default is overwriting, but it is the current behaviour so we should not change it

Also, can you change the parameters name from fix_coord_bounds to overwrite_coord_bnds. The current name is a bit misleading, as the bounds will be fixed anyway if they do not exist

@valeriupredoi
Copy link
Contributor Author

I do not like that the default is overwriting, but it is the current behaviour so we should not change it

Also, can you change the parameters name from fix_coord_bounds to overwrite_coord_bnds. The current name is a bit misleading, as the bounds will be fixed anyway if they do not exist

yeah good suggestion, will do now 👍

@valeriupredoi valeriupredoi merged commit 89140c4 into master Feb 16, 2021
@valeriupredoi valeriupredoi deleted the allow_user_turn_off_fixing_bounds_cmorizers branch February 16, 2021 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants