search

digitalearthpacific / dep-tools

Processing tools for Digital Earth Pacific
MIT License
1 stars 0 forks source link

Fix STAC writing and add AWS writing functions #53

Open alexgleith opened 6 months ago

alexgleith commented 6 months ago

Refactor STAC writing, as I don't think it was working before. Was only writing a single asset.

Fixes path returns from the writer.

Also implements an S3 writer, including all the existence checks and so on.

I think this is ready for merging, although I could clean up the commits a bit!

alexgleith commented 6 months ago

Ok, rebased and cleaned up the commits.

alexgleith commented 5 months ago

Hey @jessjaco the only failing test now is this one:

FAILED tests/test_landsat_mspc_stac_search_fixes.py::test_search_for_stac_items_with_bad_geoms - assert 12 == 24

And I haven't fixed it yet because I don't know what the correct answer is. Presumably the result should be 24, but as it does an actual search of the MSPC API, it's hard to know, as they might be returning less items.

jessjaco commented 5 months ago

Hey @jessjaco the only failing test now is this one:

FAILED tests/test_landsat_mspc_stac_search_fixes.py::test_search_for_stac_items_with_bad_geoms - assert 12 == 24

And I haven't fixed it yet because I don't know what the correct answer is. Presumably the result should be 24, but as it does an actual search of the MSPC API, it's hard to know, as they might be returning less items.

It's from your own issue, as I commented in the code: https://github.com/microsoft/PlanetaryComputer/issues/296

edit: Ok, I admit that 24 is a magic number not inferred from the issue, but this is the situation I was trying to repair. I welcome a more robust test.

alexgleith commented 5 months ago

It's from your own issue, as I commented in the code

Sorry, not trying to blame here! Just saying I didn't fix it by making it 12 because I wanted to investigate :)

jessjaco commented 5 months ago

It's from your own issue, as I commented in the code

Sorry, not trying to blame here! Just saying I didn't fix it by making it 12 because I wanted to investigate :)

Now that I look at it, why did you make it 24?

alexgleith commented 5 months ago

Now that I look at it, why did you make it 24?

Oh dear, it does appear I'm to blame!

alexgleith commented 5 months ago

Rebased and fixed the conflict, while also fixing that test (doh!)

Ready for review and potentially merging, @jessjaco.

alexgleith commented 1 month ago

If we can import the grids, then we have a simple code-based definition to work with. I think it's sensible to include them here, as they relate to our work using dep-tools, and it's simpler than having another python project to load over in the grids repo.

I don't really mind though, so if they're in dep-grids and we install them there, that's fine too.

(Unrelated, but I do think we need to pick a different projection still... one that's equal area!)