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

Add LAC support #45

Merged
merged 13 commits into from
Dec 5, 2019
Merged

Add LAC support #45

merged 13 commits into from
Dec 5, 2019

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Dec 3, 2019

This PR adds support for LAC reading.

Hopefully it keeps the GAC reading untouched.

This supersedes and closes #5

@mraspaud mraspaud requested a review from sfinkens December 3, 2019 17:54
@mraspaud mraspaud self-assigned this Dec 3, 2019
Copy link
Member

@sfinkens sfinkens left a comment

Choose a reason for hiding this comment

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

Nice job! I didn't go through all the details, just two questions:

  • The KLM readers don't have a scan_points attribute, is that correct?
  • For POD, the scan_points array only has 408 elements, is that correct?
  • That said, it would be nice if you could explain the meaning of scan_points and offset in the reader docstring :)

@mraspaud
Copy link
Member Author

mraspaud commented Dec 4, 2019

Good points, I'll check these

Comment on lines +55 to +58
'noaa9': {'roll': 0.000,
'pitch': 0.0025,
'yaw': 0.000,
},
Copy link
Member Author

Choose a reason for hiding this comment

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

@sfinkens could this be a problem ?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, that's the reason!

Copy link
Member Author

Choose a reason for hiding this comment

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

So now we just have to decide if it's making the data better or not, right ? or should I just remove this ?

@mraspaud
Copy link
Member Author

mraspaud commented Dec 4, 2019

@sfinkens

  1. scan_points is only used for computing new navigation points for the clock drift correction. So KLM shouldn't need them.
np.arange(3.5, 2048, 5).shape                                                                                                        
(409,)

?
Adding some documentation now

@sfinkens
Copy link
Member

sfinkens commented Dec 4, 2019

Sorry about the 408, I printed np.diff(np.arange(...)).shape, that's why it was one element shorter

Copy link
Member

@sfinkens sfinkens left a comment

Choose a reason for hiding this comment

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

System tests pass 👍

@mraspaud mraspaud merged commit 19c82fc into pytroll:master Dec 5, 2019
@mraspaud mraspaud deleted the feature-lac2 branch December 5, 2019 09:59
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.

3 participants