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

Change S-2 processor to have harmonize_to_old option #51

Merged
merged 3 commits into from
Feb 14, 2024

Conversation

alexgleith
Copy link
Contributor

Tweaked the harmonize_to_old function so that it handles data without the band dimension.

Changed the s-2 processor so that there are two flags, one for scale_and_offset and another for harmonize_to_old.

Array histogram before:

image

And after:

image

@alexgleith alexgleith requested a review from jessjaco February 12, 2024 23:31
@alexgleith alexgleith force-pushed the rename-s2-offset-to-harmonize branch from fa12330 to 8830480 Compare February 13, 2024 23:01
@alexgleith
Copy link
Contributor Author

Any issues with me merging this, @jessjaco?

@jessjaco
Copy link
Collaborator

  1. I'd prefer making changes to the develop branch if possible. I don't have the bandwidth to track changes across multiple branches.
  2. I'm concerned about the change to SimpleLoggingAreaTask. I don't currently use it but it is limiting processing to datasets that have a time variable. What about doing this step in the processor?
  3. I think I follow the changes in the harmonization - they're to handle datasets? I'd probably just cast to / from a dataset and recast at the end so there's not duplicate logic

@alexgleith
Copy link
Contributor Author

I'd prefer making changes to the develop branch if possible. I don't have the bandwidth

I don't want to work of the develop branch yet, as I already have code that works off the main branch. Will refactor, but don't want to yet. I'd prefer to get this merged into main and for us to handle the changes from develop separately. I too don't want to track changes in multiple branches!

I'm concerned about the change to SimpleLoggingAreaTask.

Yeah, good point. I'll have a look at how to incorporate that into the processor.

I'd probably just cast to / from a dataset and recast at the end so there's not duplicate logic

I guess so... I'll look at that too.

@alexgleith
Copy link
Contributor Author

I think this resolves concerns 2 and 3, @jessjaco.

Can we compromise on 1 please?

@jessjaco jessjaco merged commit bb5df37 into main Feb 14, 2024
2 checks passed
@jessjaco jessjaco deleted the rename-s2-offset-to-harmonize branch February 14, 2024 23:10
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.

2 participants