Use Sensor Gain Adjustment for DJI sensors when calibration panels are not present/used#69
Use Sensor Gain Adjustment for DJI sensors when calibration panels are not present/used#69joeyfranck56 merged 15 commits intomainfrom
Conversation
joeyfranck56
changed the title
zthorson
left a comment
zthorson
left a comment
There was a problem hiding this comment.
I've never really used github workflows so I learned something new with this PR. I just added a manual trigger so I could understand them better (Can be triggered with github cli via: gh workflow run "executable_build_test.yaml" --ref dji-band-sensitivity then watched via gh watch and appears to complete successfully.
Though it doesn't look like you can access the exe's, with the current runner, it does at least test them.
| elif "drone-dji:SensorGainAdjustment" in xmp: | ||
| gain_adj = float(xmp["drone-dji:SensorGainAdjustment"]) | ||
| return gain_adj |
There was a problem hiding this comment.
Seems good to me. I can't think of any case where we would have Camera:BandSensitivity and drone-dji:SensorGainAdjustment at the same time, so we shouldn't have to worry about the order of the if statements.
| "Skipping %s images because they don't have data for all bands", | ||
| images_before_filtering - len(new_image_df.index), |
There was a problem hiding this comment.
I find f-strings a bit easier to read for most cases, but it really doesn't matter here.
There was a problem hiding this comment.
Agreed, for some reason the linter doesn't like using f strings with logger calls
| pathex=['.'], | ||
| binaries=[], | ||
| datas=[('exiftool/exiftool.exe', '.')], | ||
| datas=[('exiftool/exiftool.exe', '.')] + metadata_datas, |
There was a problem hiding this comment.
I'd expect the onefile and the dir versions to have the same datas list, but it looks like there aren't related exiftool cfg folders ( ('cfg/exiftool.cfg', 'cfg'), ('cfg/reg_config.ini', 'cfg'), ('sentera_radiometric_corrections_icon.ico', '.') that you had below. Are these files needed for the exe?
| pathex=['.'], | ||
| binaries=[], | ||
| datas=[('exiftool/exiftool.exe', '.'), ('cfg/exiftool.cfg', 'cfg'), ('cfg/reg_config.ini', 'cfg')], | ||
| datas=[('exiftool/exiftool.exe', '.'), ('cfg/exiftool.cfg', 'cfg'), ('cfg/reg_config.ini', 'cfg')] + metadata_datas, |
There was a problem hiding this comment.
Same comment as above, are these extra files (exiftool cfg files) needed in both the onefile and dir version? I'd expect if it's needed in one it would be needed for the others?
| tqdm = "^4.59.0" | ||
| opencv-contrib-python = ">=4.6.0,<4.7.0" | ||
| imgparse = {git = "https://github.com/SenteraLLC/py-image-metadata-parser.git", tag = "v2.0.4"} | ||
| imgparse = {git = "https://github.com/SenteraLLC/py-image-metadata-parser.git", tag = "v2.0.10"} |
There was a problem hiding this comment.
I think this should still be good. We've had issues with conflicting versions of py-image-metadata-parser for quicktile-ms-wrapper, but I think elliot was able to resolve them in quicktile-core instead of here:
https://github.com/SenteraLLC/quicktile-ms-wrapper/pull/26
https://github.com/SenteraLLC/quicktile-core/pull/86
If we aren't planning on rolling this updated version of py-radiometric-corrections into the quicktile-ms-wrapper then we can ignore this entirely for now.
2 participants
DJI Band Sensitivity
What?
Why?
PR Checklist
Breaking Changes