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

[WIP] Add MIRS file reader #1285

Closed
wants to merge 3 commits into from

Conversation

katherinekolman
Copy link

This adds a reader for files from MIRS.

  • Tests added
  • Tests passed
  • Passes flake8 satpy
  • Fully documented
  • Add your name to AUTHORS.md if not there already

@ghost
Copy link

ghost commented Jul 31, 2020

DeepCode's analysis on #b990f9 found:

  • ⚠️ 1 warning 👇

Top issues

Description Example fixes
Missing close for open, add close or use a with block. Occurrences: 🔧 Example fixes

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 89.794% when pulling b990f9a on katherinekolman:mirs_reader into f80c568 on pytroll:master.

@codecov
Copy link

codecov bot commented Jul 31, 2020

Codecov Report

Merging #1285 into master will decrease coverage by 0.28%.
The diff coverage is 18.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1285      +/-   ##
==========================================
- Coverage   90.07%   89.79%   -0.29%     
==========================================
  Files         218      219       +1     
  Lines       31580    31707     +127     
==========================================
+ Hits        28447    28471      +24     
- Misses       3133     3236     +103     
Impacted Files Coverage Δ
satpy/readers/mirs.py 18.89% <18.89%> (ø)

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 f80c568...b990f9a. Read the comment docs.

@joleenf joleenf mentioned this pull request Dec 15, 2020
5 tasks
joleenf added a commit to joleenf/satpy that referenced this pull request Jan 22, 2021
@joleenf joleenf mentioned this pull request Jan 22, 2021
@joleenf
Copy link
Contributor

joleenf commented Feb 27, 2021

@djhoese In this commit, the file metadata is taking precedence over metadata in the yaml for the metop file used for testing. (IMG_SX.M2.D17037.S1601.E1607.B0000001.WE.HR.ORB.nc). Therefore, standard_name is added when reading the file metadata. I thought this line in the available_datasets meant that it is possible to append new information to the metadata:

This method is not guaranteed that it will be called before any other file type’s handler. The availability “boolean” not being None does not mean that a file handler called later can’t provide an additional dataset, but it must provide more identifying (DataID) information to do so and should yield its new dataset in addition to the previous one.

Can the boolean be True, False or None? Or is it only either True or None? I think the available_dataset() information above is saying, "Even if the availability "boolean" is True rather than None, a file handler could provide additional datasets." After that line, what is the more identifying DataID? Is this referring to the more definitive DataIDs like Brightness Temperature data defined by frequency and polarization "btemp_23v" versus the wider scope of BT? If so, then that is working correctly in this reader, because both the BT and the definitive brightness temperature channels are being yielded. Is there a way to keep variable metadata from a yaml and use metadata from a file, or is it either/or?
Also, in "Migrating to Xarray" the example shows using the "coord" attribute. Is assign_coords the only concern in this issue report?

@joleenf
Copy link
Contributor

joleenf commented Feb 27, 2021

@djhoese Previous comment was supposed to be under #1511

@djhoese
Copy link
Member

djhoese commented Jun 2, 2021

Replaced by #1511

@djhoese djhoese closed this Jun 2, 2021
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.

4 participants