fraymio / modis-tools

Tools for working with the MODIS API and MODIS data.
Apache License 2.0
22 stars 11 forks source link

Ds 6157 url issue with modisa l3b chl ms #42

Closed mylesstokowski closed 9 months ago

mylesstokowski commented 9 months ago

Description

This PR is based on #41 and contains:

Closes # (issue)

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

Checklist:

Next Steps

On behalf of the Modis Tools Dev Team, thank you for your hard work! ✨

mylesstokowski commented 9 months ago

Should we also include the always arg?

Interesting, I wasn't familiar. I tried adding that parameter, and didn't observe any chance in behavior. I tested by running test_space_is_removed_from_link() as is and with the href field removed in the input. Since it's working now, I'm tempted to leave it as is.

mylesstokowski commented 9 months ago

Thanks for the review @cearlefraym! One q: any preference whether I merge this into your base branch, then merge that to main, or alternatively switch the base branch to main and merge it directly?

And relatedly, confirming there's no special approval process for this repo (i.e., confirming that we don't need another approval to merge to main)?

mylesstokowski commented 9 months ago

@cearlefraym I pointed it to main, reviewed the changes in your branch, made a couple suggestions, and removed the test file. Take a look, then I'll test once more and merge.