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

feat: nisar sim v2 #8

Merged
merged 7 commits into from
Mar 6, 2024
Merged

feat: nisar sim v2 #8

merged 7 commits into from
Mar 6, 2024

Conversation

jjfrench
Copy link
Collaborator

@jjfrench jjfrench commented Mar 4, 2024

Before you submit a pull request, please fill in the following:

Description:
Slimming down the NISAR-sim package to reflect the latest simulated NISAR data.

Removed:

  • requirements-dev.txt
  • setup.cfg

Modified:

  • pyproject.toml
    • moved everything over from the setup.cfg and updated some of the configurations such as using ruff and some of the dependencies.
    • ensures that some packages get installed by listing them in the dependencies here
  • Dockerfile
    • consolidated down to one virtual environment, when trying to run the commands I kept seeing that packages were missing but the base environment had most of the packages installed that I needed. The commands were being triggered from one environment and the installs were happening in another

This latest simulated NISAR package is based on the latest simulated NISAR product suite: https://nisar.jpl.nasa.gov/data/sample-data/, where there are 10 different data products with relatively the same h5 structure (with the exception of Soil Moisture).

PR checklist:

  • Code is formatted (run scripts/format).
  • Code lints properly (run scripts/lint).
  • Tests pass (run scripts/test).
  • Documentation has been updated to reflect changes, if applicable.
  • Examples have been updated to reflect changes, if applicable
  • Changes are added to the CHANGELOG. (Was never released - No changes)

@jjfrench jjfrench requested a review from emileten March 4, 2024 15:30
Copy link

@omshinde omshinde left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you for your efforts.

  1. Probably for later versions, we might need to add properties from here: https://github.com/stac-extensions/sar. Specifically, product type, frequency, etc.
  2. Also, is RRSD a good key name for a general audience? I am not sure if the asset names should target a general audience or domain scientists but RRSD is difficult to interpret IMO.

Copy link
Collaborator

@emileten emileten left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @jjfrench

I have a couple questions that came up while reading this. Maybe they could be briefly addressed if you provide a brief description of the changes made in this PR. I see two general changes, that you could maybe shed light on with just a couple lines in the description.

  • dependencies management (see my comments)
  • application changes (changes to src/stactools/nisar_sim and related changes in tests)

Thanks !

@jjfrench
Copy link
Collaborator Author

jjfrench commented Mar 5, 2024

Thanks for the feedback @emileten, let me know if you have any other questions! Feel free to resolve conversations if I was able to answer your question.

@emileten
Copy link
Collaborator

emileten commented Mar 6, 2024

Thanks so much for the explanations @jjfrench! Feel free to merge, I resolved the conversations.

@jjfrench jjfrench merged commit 2b6dd48 into main Mar 6, 2024
4 checks passed
@jjfrench jjfrench deleted the feat/nisar-sim_v2 branch March 6, 2024 14:44
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.

3 participants