PPS 2021 refactor #60
Conversation
…4oldfiles, create_xml_timestat_from_scene, get_time_control_ascii_filename, get_time_control_ascii_filename_candidates, get_product_statistics_files
get_sceneid create_pps_call_command_sequence get_pps_inputfile get_xml_outputfiles
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
==========================================
+ Coverage 70.55% 74.24% +3.68%
==========================================
Files 15 15
Lines 2201 1778 -423
Branches 296 266 -30
==========================================
- Hits 1553 1320 -233
+ Misses 606 419 -187
+ Partials 42 39 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Added some tests
There was a problem hiding this comment.
Just couple of quick comments:
pps_runner.pyis deleted andpps2018_runner.pyis modified although the description says v2014 and v2018 support are dropped. Shouldn't it be the other way around?- the example configs for v2014 and v2018 should be deleted for clarity if they are not supported any more
setup.pyinstalls the v2018 runner
|
Thanks for the comments @pnuu! The pps_runner.py was for version 2014 only. The pps2018_runner.py was for version 2018 and forward. I can rename the pps2018_runner.py to pps_runner.py as there is now only one runner. I will have a look at the config files too. |
Had to remove the timeout from the subscr.recv, not sure if it is a problem.
| @@ -51,7 +51,7 @@ def run(self): | |||
| LOG.debug("Subscribe topics = %s", str(self.subscribe_topics)) | |||
| with posttroll.subscriber.Subscribe("", self.subscribe_topics, True) as subscr: | |||
|
|
|||
| for msg in subscr.recv(timeout=90): | |||
| for msg in subscr.recv(): | |||
There was a problem hiding this comment.
Not sure why we had a timeout of 90 seconds before, but is it safe to change this? Why was this needed?
There was a problem hiding this comment.
I removed it because the patched subscriber did not have the timeout keyword. I am not sure if it is safe to remove.
There was a problem hiding this comment.
The timeout is needed I think when the thread is shut down. Maybe something we need in the patched_subscriber then.
There was a problem hiding this comment.
Ok, I'm not sure I really understood how this module was working before, so I trust that you have tested the functionality to be confident this is working.
It's really nice with all the code removal, it will make the code easier to read. And good with new tests also!
bin/run_nwp_preparation.py
Outdated
|
|
||
| LOG = logging.getLogger('nwp-preparation') | ||
|
|
||
| # datetime.datetime.utcnow => datetime.datetime.now(datetime.UTC) ~python 3.12 |
There was a problem hiding this comment.
Yes, the datetime.datetime.now(datetime.UTC) is working in python 3.12 but in 3.9 I get AttributeError: module 'datetime' has no attribute 'UTC'. The datetime.datetime.now(datetime.UTC) will also need some changes in other parts of the code as it is not expecting timezone-aware objects. Currently I get a warning in 3.12: DeprecationWarning: datetime.datetime.utcnow() is deprecated and scheduled for removal in a future version. Use timezone-aware objects to represent datetimes in UTC: datetime.datetime.now(datetime.UTC).
| @@ -51,7 +51,7 @@ def run(self): | |||
| LOG.debug("Subscribe topics = %s", str(self.subscribe_topics)) | |||
| with posttroll.subscriber.Subscribe("", self.subscribe_topics, True) as subscr: | |||
|
|
|||
| for msg in subscr.recv(timeout=90): | |||
| for msg in subscr.recv(): | |||
There was a problem hiding this comment.
The timeout is needed I think when the thread is shut down. Maybe something we need in the patched_subscriber then.
| @@ -38,102 +39,111 @@ | |||
| datefmt='%H:%M:%S') | |||
|
|
|||
|
|
|||
| class NWPprepareRunner(unittest.TestCase): | |||
| @pytest.fixture | |||
Also allow to test with list of files
Needed for threads that are shut down. For now it is not working with the patched reciever
Co-authored-by: Adam Dybbroe <[email protected]>
utcnow to be depricated
There is a logreader function also in the utils module.
Update the code to support only PPS-2021 and drop support for PPS 2014. The main thing is that the pps runner need only the level1c file to run. The NWP preparation is now in a separate runner. Some refactoring, with more subroutines in prepare_nwp
pytest nwcsafpps_runnerflake8 nwcsafpps_runner