search

google / xarray-beam

Distributed Xarray with Apache Beam
https://xarray-beam.readthedocs.io
Apache License 2.0
134 stars 7 forks source link

Created ValidateEachChunk transformation. #25

Closed alxmrs closed 3 years ago

alxmrs commented 3 years ago

New beam transformation allows users to validate that chunk keys match datasets during pipeline execution. These can be interspersed in pipelines to provide sanity checks.

alxmrs commented 3 years ago

Looks great, can you add a couple of basic tests, too?

Oh man! I seem to have lost my tests – a casualty of my attempt to fix the commit history with rebase. Sorry about that, I will re write them now.

alxmrs commented 3 years ago

On this note: what is the convention with git history for this repo? Do you prefer merging single commits to make the history cleaner? Or, is it ok to squash multiple commits into one?

shoyer commented 3 years ago

On this note: what is the convention with git history for this repo? Do you prefer merging single commits to make the history cleaner? Or, is it ok to squash multiple commits into one?

In the process of review, you should push new commits for each set of changes so they are easier to review.

I normally like squash & merge, but copybara doesn't support it. I don't really think it's worth the hassle of manual squashing, unless you end up with many tens of commits. Just try to keep to ~1 commit per review cycle.

alxmrs commented 3 years ago

Thanks! I like that reasoning. I will keep that in mind going forward.