opera-adt / COMPASS

COregistered Multi-temPorAl Sar Slc
Apache License 2.0
38 stars 18 forks source link

bug fixes in `gdal.Open` and `validate_product.py` #164

Closed LiangJYu closed 1 year ago

LiangJYu commented 1 year ago

This PR:

seongsujeong commented 1 year ago

@LiangJYu In the context of providing information debugging, what do you think about printing out the burst ID to the logger? maybe somewhere in the lines below?

https://github.com/opera-adt/COMPASS/blob/643b951efb0f2763b104163365fae7515f52d6f4/src/compass/s1_geocode_slc.py#L95-L98

LiangJYu commented 1 year ago

@LiangJYu In the context of providing information debugging, what do you think about printing out the burst ID to the logger? maybe somewhere in the lines below?

https://github.com/opera-adt/COMPASS/blob/643b951efb0f2763b104163365fae7515f52d6f4/src/compass/s1_geocode_slc.py#L95-L98

I like the idea. I don't immediately see a clean way of doing it though. Will stew on this...

LiangJYu commented 1 year ago

@LiangJYu In the context of providing information debugging, what do you think about printing out the burst ID to the logger? maybe somewhere in the lines below? https://github.com/opera-adt/COMPASS/blob/643b951efb0f2763b104163365fae7515f52d6f4/src/compass/s1_geocode_slc.py#L95-L98

I like the idea. I don't immediately see a clean way of doing it though. Will stew on this...

@seongsujeong rather than passing around the burst ID so open_raster what do you think about adding the following call somewhere after the block you linked above?

info_channel.log(f"Starting geocoding of {burst_id} for {date_str}")

This keeps open_raster general while adding log info that can facilitate log readability.

seongsujeong commented 1 year ago

@LiangJYu In the context of providing information debugging, what do you think about printing out the burst ID to the logger? maybe somewhere in the lines below? https://github.com/opera-adt/COMPASS/blob/643b951efb0f2763b104163365fae7515f52d6f4/src/compass/s1_geocode_slc.py#L95-L98

I like the idea. I don't immediately see a clean way of doing it though. Will stew on this...

@seongsujeong rather than passing around the burst ID so open_raster what do you think about adding the following call somewhere after the block you linked above?

info_channel.log(f"Starting geocoding of {burst_id} for {date_str}")

This keeps open_raster general while adding log info that can facilitate log readability.

I like that implementation. Thanks :)

seongsujeong commented 1 year ago

Not sure if you like the idea. But if we can exploit this PR for the last fix, can we comment out or remove the couple of lines in validate_product.py? I've found that this is necessary for the static layer products from CAL/VAL SAS.

https://github.com/opera-adt/COMPASS/blob/643b951efb0f2763b104163365fae7515f52d6f4/src/compass/utils/validate_product.py#L58-L59

LiangJYu commented 1 year ago

Not sure if you like the idea. But if we can exploit this PR for the last fix, can we comment out or remove the couple of lines in validate_product.py? I've found that this is necessary for the static layer products from CAL/VAL SAS.

https://github.com/opera-adt/COMPASS/blob/643b951efb0f2763b104163365fae7515f52d6f4/src/compass/utils/validate_product.py#L58-L59

Done. PR title edited reflect expanded scope.