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

Update CMR query #21

Merged
merged 1 commit into from
May 3, 2022
Merged

Update CMR query #21

merged 1 commit into from
May 3, 2022

Conversation

sssangha
Copy link
Contributor

@sssangha sssangha commented May 3, 2022

Update deployment notebooks with CMR query fixes, following the implementation from @asjohnston-asf

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@cmarshak
Copy link
Contributor

cmarshak commented May 3, 2022

Hey Sim - where are these functions taken from? Have they been tested?

Can you explain what the issue is with the asf-search type query I am doing now?

I am not sure these will work as you expect them to given that the reference and secondary fields are not written correctly to GUNW products: ACCESS-Cloud-Based-InSAR/DockerizedTopsApp#48

Maybe I am wrong - hope so!

@jhkennedy jhkennedy requested a review from asjohnston-asf May 3, 2022 17:10
@asjohnston-asf
Copy link
Member

I had drafted get_cmr_products() and parse_echo10() and shared them via Slack a week or two ago. ASF was hoping to set aside time next week to help integrate them into the notebook, but I can probably take a couple minutes today to help review if needed.

@sssangha
Copy link
Contributor Author

sssangha commented May 3, 2022

@cmarshak the previous implementation was passing an empty list after the CMR query.

But with Andrew's implementation, I had confirmed at least for one case this works as intended by filtering out existing products. I could run some more sanity checks though.

@sssangha sssangha merged commit 4ee4061 into main May 3, 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.

3 participants