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

Concatenate weighted using float and new example xtf to tiff #41

Merged
merged 4 commits into from
Jul 3, 2024

Conversation

joakimsk
Copy link
Contributor

Concatenate weighted using float internally and example of writing xtf to tiff. Closes issue #39.

@oysstu
Copy link
Owner

oysstu commented Jul 2, 2024

Thanks for the PR. I don't have any XTF files where the weight parameter is used. Have you been able to verify that it works as expected? E.g. on an edgetech sonar or similar.

Also, good idea to add an example. Do you think it might be better to make it xtf to png/jpeg or something instead of tiff? Although tiff is just another image format, some people might think it is a geotiff example.

@joakimsk
Copy link
Contributor Author

joakimsk commented Jul 2, 2024

Thanks for the PR. I don't have any XTF files where the weight parameter is used. Have you been able to verify that it works as expected? E.g. on an edgetech sonar or similar.

Also, good idea to add an example. Do you think it might be better to make it xtf to png/jpeg or something instead of tiff? Although tiff is just another image format, some people might think it is a geotiff example.

You are very welcome, thank you for creating pyxtf!

I have Kongsberg HiSAS 2040 XTF data that uses weighting, you can find XTFs here to try out if you would like:
https://github.com/joakimsk/hisas2040-tools

Besides that, I do not have other XTFs at hand, sadly, but for my use with HiSAS 2040, it works well.

PNG may be a better option, my goal was just to make it store without compression, till we get to a geotiff implementation. Do you want to just rename it from .tiff to .png in output, or do you want me to make a new PR? (First time doing this)

@oysstu
Copy link
Owner

oysstu commented Jul 2, 2024

Ah, yes. It's been a while, but I think I implemented the weight parameter when working with some hisas 1030 data. Triton used to have some demo files on their webpage, so I thought maybe I had used those.

Strictly speaking, it makes more sense to implement raising to the power of 2 through bit-shifts instead of multiplication for unsigned arrays, but I implemented it like this originally to be compatible with xtf files using float32 sample data type.

No need to make a new PR, just push more commits to your branch. Png is also a lossless format, but yes, it has some compression. What if you just rename the file to xtf_to_image.py? That should remove any doubt about what it does.

@joakimsk
Copy link
Contributor Author

joakimsk commented Jul 2, 2024

Ah, yes. It's been a while, but I think I implemented the weight parameter when working with some hisas 1030 data. Triton used to have some demo files on their webpage, so I thought maybe I had used those.

Strictly speaking, it makes more sense to implement raising to the power of 2 through bit-shifts instead of multiplication for unsigned arrays, but I implemented it like this originally to be compatible with xtf files using float32 sample data type.

No need to make a new PR, just push more commits to your branch. Png is also a lossless format, but yes, it has some compression. What if you just rename the file to xtf_to_image.py? That should remove any doubt about what it does.

Very well, I guess as long as it works, multiplication may be easier to read at least. I have modified to write png, and added a header now. Does that look ok?

@oysstu oysstu merged commit e750c72 into oysstu:master Jul 3, 2024
@oysstu
Copy link
Owner

oysstu commented Jul 3, 2024

Great, thank you for your contribution 👍

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.

None yet

2 participants