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

Develop satellite #71

Merged
merged 38 commits into from
Aug 16, 2022
Merged

Develop satellite #71

merged 38 commits into from
Aug 16, 2022

Conversation

dwfncar
Copy link
Contributor

@dwfncar dwfncar commented Jul 25, 2022

No description provided.

@zmoon zmoon self-requested a review July 25, 2022 17:06
@zmoon
Copy link
Member

zmoon commented Jul 25, 2022

Hi @dwfncar, I resolved the merge conflict but am not able to push. Maybe if you "Allow edits by maintainers" in the PR somewhere?

(Note that develop and stable in the NCAR fork are a bit behind.)

@dwfncar
Copy link
Contributor Author

dwfncar commented Jul 26, 2022 via email

@zmoon
Copy link
Member

zmoon commented Jul 26, 2022

@dwfncar it is supposed to be like this but maybe it is different since the NCAR fork is under an org not a personal account

@dwfncar
Copy link
Contributor Author

dwfncar commented Jul 26, 2022 via email

Copy link
Member

@zmoon zmoon left a comment

Choose a reason for hiding this comment

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

Initial comments:

  • can you modify the hdfio test so that the test file is deleted after it is complete?
  • pyhdf is currently not a required dep and I would prefer to keep it that way. Possible resolutions include:
    • modify the hdfio module so that the module-level import is not needed (within functions, you could use the _import_required helper seen in geoms reader)
  • if hdfio is going to be the only module, we could probably remove the hdf directory
  • for the new readers, is it possible to get tiny test files so that we can test them?
    • if not, please prepend _ and append mm to the module names

monetio/models/raqms.py Outdated Show resolved Hide resolved
monetio/models/raqms.py Outdated Show resolved Hide resolved
…it__.py files. Modified test_hdf.py to delete test.hdf file; not sure why this could not be removed within tearDown.
monetio/sat/__init__.py Outdated Show resolved Hide resolved
tests/test_hdfio.py Outdated Show resolved Hide resolved
@zmoon
Copy link
Member

zmoon commented Jul 29, 2022

Is fv3raqms reader supposed to be in this PR?

@mebruckner
Copy link
Contributor

mebruckner commented Jul 29, 2022 via email

@zmoon
Copy link
Member

zmoon commented Aug 16, 2022

omps_limb removed on request of @mebruckner

Copy link
Member

@zmoon zmoon left a comment

Choose a reason for hiding this comment

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

I didn't look at the details of the two modules we are leaving "hidden". If at some point we want to make them public in monetio I will take a closer look.

@zmoon zmoon merged commit 8841c8a into noaa-oar-arl:develop Aug 16, 2022
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.

None yet

4 participants