Skip to content

Conversation

@vpipkt
Copy link
Member

@vpipkt vpipkt commented Nov 11, 2019

  • function reference docs for new functions
  • unit test for SQL snip in docs
  • expose extract and mask functions in python api
  • ensure python and scala api's are equivalent
  • consider extract bits input and output cell types, unit tests
  • update release notes
  • Fix mask by value function incorrectly interprets 0 #416
  • Reorganize masking stuff in RasterFucntionsSpec
  • update the masking page to show landsat mask using mask_by_bits

Signed-off-by: Jason T. Brown <jason@astraea.earth>
Signed-off-by: Jason T. Brown <jason@astraea.earth>
Mask by value of 0 fixed
Masking cell type mutations in rf_mask_by_values is resolved
Masking by extraction of bits implemented

Signed-off-by: Jason T. Brown <jason@astraea.earth>
…ests

Signed-off-by: Jason T. Brown <jason@astraea.earth>
Signed-off-by: Jason T. Brown <jason@astraea.earth>
Signed-off-by: Jason T. Brown <jason@astraea.earth>
@vpipkt vpipkt marked this pull request as ready for review November 15, 2019 18:30
@vpipkt vpipkt requested a review from metasim November 15, 2019 18:30
@courtney-layman
Copy link
Contributor

I read through the masking doc. I think it looks great!

Copy link
Member

@metasim metasim left a comment

Choose a reason for hiding this comment

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

There are a few minor change requests. But don't delay because of that; it's so amazing I want to get it merged ASAP.


Applies a mask from blacklisted bit values in the `mask_tile`. Working from the right, the bits from `start_bit` to `start_bit + num_bits` are @ref:[extracted](reference.md#rf_local_extract_bits) from cell values of the `mask_tile`. In all locations where these are in the `mask_values`, the returned tile is set to NoData; otherwise the original `tile` cell value is returned.

This function is not available in the SQL API. The below is equivalent:
Copy link
Member

Choose a reason for hiding this comment

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

You'll have to explain to me why... If it's array creation, it should be available with the array(x, y, z, ...) function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically I threw in the towel on trying to create an Expression with 5 arguments (tile, tile, int, int, array) ... If you have any intiution on this it's welcome! Maybe we can file another issue to bring that along.

l8_df.select('data', 'mask', 'data_masked', 'data2', rf_extent('data_masked')) \
.filter(rf_data_cells('data_masked') > 32000)
```

Copy link
Member

Choose a reason for hiding this comment

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

Wow. That's all I can say. This is absolutely brilliant.

long_trip = df.first()["tile"]
self.assertEqual(long_trip, a_tile)

def test_masked_deser(self):
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand what this is testing...

Copy link
Member Author

Choose a reason for hiding this comment

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

basically just testing that a Tile that gets deserialized has the expected masking on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically is the same thing as my commented out block at RasterFunctionsTest.py lines 282 on.... paranoia

# self.assertEqual(mask_fill_tile.cells.mask.sum(), 4,
# f'Expected {16 - 4} data values but got the masked tile:'
# f'{mask_fill_tile}'
# )
Copy link
Member

Choose a reason for hiding this comment

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

Are you expecting mask_fill_tile.cells.mask.sum() to return 42*4*4, or the cells.sum() after the mask has been applied?

Copy link
Member

Choose a reason for hiding this comment

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

Or something else?

Copy link
Member Author

Choose a reason for hiding this comment

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

so the mask is binary where 1 indicates that location is nodata.

So by doing .mask.sum() we expect the same as rf_no_data_cells. But i kept getting this assertion failing locally.

Copy link
Member Author

Choose a reason for hiding this comment

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

so the mask is binary where 1 indicates that location is nodata.

So by doing .mask.sum() we expect the same as rf_no_data_cells. But i kept getting this assertion failing locally.

Signed-off-by: Jason T. Brown <jason@astraea.earth>
…deserialization

Signed-off-by: Jason T. Brown <jason@astraea.earth>
@vpipkt vpipkt merged commit b396400 into locationtech:develop Nov 18, 2019
@vpipkt vpipkt deleted the feature/mask_by_bits branch November 18, 2019 19:00
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