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

Factorize header definitions between hrit_msg and native_msg. Fix a bug in header definition. #301

Merged
merged 10 commits into from
May 29, 2018

Conversation

sjoro
Copy link
Collaborator

@sjoro sjoro commented May 18, 2018

Fix a bug in hrit_msg and native_msg header-definitions. Factorize header-definitions between hrit_msg and native_msg. Add image trailer reading for native_msg. Clean-up code.

  • Tests added
  • Tests passed
  • Passes git diff origin/master **/*py | flake8 --diff

@coveralls
Copy link

coveralls commented May 18, 2018

Coverage Status

Coverage increased (+0.5%) to 66.345% when pulling a424fc6 on sjoro:master into 89217cb on pytroll:master.

@codecov
Copy link

codecov bot commented May 18, 2018

Codecov Report

Merging #301 into master will increase coverage by 0.47%.
The diff coverage is 39.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #301      +/-   ##
==========================================
+ Coverage   65.86%   66.34%   +0.47%     
==========================================
  Files         118      119       +1     
  Lines       14336    14414      +78     
==========================================
+ Hits         9443     9563     +120     
+ Misses       4893     4851      -42
Impacted Files Coverage Δ
satpy/readers/msg_base.py 96.96% <ø> (ø) ⬆️
satpy/tests/reader_tests/test_msg_base.py 84.61% <ø> (ø) ⬆️
satpy/tests/reader_tests/test_eum_base.py 0% <0%> (ø)
satpy/tests/reader_tests/test_native_msg.py 85.71% <100%> (-0.5%) ⬇️
satpy/tests/reader_tests/test_hrit_goes.py 75.32% <100%> (ø) ⬆️
satpy/readers/native_msg_hdr.py 100% <100%> (+71.02%) ⬆️
satpy/readers/hrit_base.py 81.41% <100%> (+5.41%) ⬆️
satpy/readers/hrit_goes.py 50.68% <100%> (ø) ⬆️
satpy/tests/reader_tests/test_hrit_base.py 93.84% <100%> (-1.16%) ⬇️
satpy/readers/native_msg.py 15.84% <17.91%> (-2.72%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89217cb...a424fc6. Read the comment docs.

@mraspaud mraspaud added enhancement code enhancements, features, improvements component:readers labels May 21, 2018
@mraspaud mraspaud changed the title Factorize header definitions between hrit_msg and native_msg. Fix a bug in header definition. [WIP] Factorize header definitions between hrit_msg and native_msg. Fix a bug in header definition. May 21, 2018
@@ -38,16 +38,20 @@
import numpy as np

from pyresample import geometry

from satpy.readers.eum_base import (make_time_cds, time_cds_short,
Copy link
Contributor

Choose a reason for hiding this comment

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

F401 'satpy.readers.eum_base.make_time_cds' imported but unused


class CalibrationError(Exception):
pass
from satpy.readers.eum_base import make_time_cds, recarray2dict
Copy link
Contributor

Choose a reason for hiding this comment

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

F401 'satpy.readers.eum_base.make_time_cds' imported but unused

class CalibrationError(Exception):
pass
from satpy.readers.eum_base import make_time_cds, recarray2dict
from satpy.readers.msg_base import (SEVIRICalibrationHandler,
Copy link
Contributor

Choose a reason for hiding this comment

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

F401 'satpy.readers.msg_base.get_cds_time' imported but unused


data15hd = self.header['15_DATA_HEADER']
sec15hd = self.header['15_SECONDARY_PRODUCT_HEADER']

utc = data15hd['ImageAcquisition']['PlannedAcquisitionTime']
Copy link
Contributor

Choose a reason for hiding this comment

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

F841 local variable 'utc' is assigned to but never used


west = int(sec15hd['WestColumnSelectedRectangle']['Value'])
east = int(sec15hd['EastColumnSelectedRectangle']['Value'])
north = int(sec15hd["NorthLineSelectedRectangle"]['Value'])
Copy link
Contributor

Choose a reason for hiding this comment

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

F841 local variable 'north' is assigned to but never used

west = int(sec15hd['WestColumnSelectedRectangle']['Value'])
east = int(sec15hd['EastColumnSelectedRectangle']['Value'])
north = int(sec15hd["NorthLineSelectedRectangle"]['Value'])
south = int(sec15hd["SouthLineSelectedRectangle"]['Value'])
Copy link
Contributor

Choose a reason for hiding this comment

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

F841 local variable 'south' is assigned to but never used

make_time_cds_expanded)
from satpy.readers.hrit_msg import HRITMSGPrologueFileHandler
# make_time_cds_expanded)
from satpy.readers.eum_base import make_time_cds
Copy link
Contributor

Choose a reason for hiding this comment

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

F401 'satpy.readers.eum_base.make_time_cds' imported but unused

@sjoro
Copy link
Collaborator Author

sjoro commented May 25, 2018

[ x] Passes test_hrit_base, test_msg_base, test_eum_base, test_native_msg

Fails on test_hrit_goes, test_hrit_msg, need help here to fix the unittests.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

Good job !

@@ -49,6 +48,30 @@ class CalibrationError(Exception):
logger = logging.getLogger('hrit_goes')


def recarray2dict(arr):
Copy link
Member

Choose a reason for hiding this comment

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

This function should be removed, right ?

@@ -451,8 +116,6 @@ def __init__(self, filename, filename_info, filetype_info):
(msg_hdr_map,
msg_variable_length_headers,
msg_text_headers))

self.prologue = {}
Copy link
Member

Choose a reason for hiding this comment

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

instance attributes should be initialized in init

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

LGTM

@mraspaud mraspaud changed the title [WIP] Factorize header definitions between hrit_msg and native_msg. Fix a bug in header definition. Factorize header definitions between hrit_msg and native_msg. Fix a bug in header definition. May 29, 2018
@mraspaud mraspaud merged commit 93826db into pytroll:master May 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants