Gdal Plug --fid into existing GDALVectorInfo -fid optionvector info fid clean #13904
Conversation
|
Please add a test |
Co-authored-by: Even Rouault <[email protected]>
Co-authored-by: Even Rouault <[email protected]>
|
@Sionigdha Please test locally your changes before comitting, in particular the new test you've added, to avoid burning CI time that is shared with other contributors. |
|
@rouault Thanks for the feedback. I’ll run the new test locally and make sure everything passes before pushing further updates. |
|
I attempted to set up a full local GDAL build on Windows but wasn’t able to complete it yet. I’ve reviewed the implementation carefully, aligned it with existing argument patterns, and fixed CI issues iteratively. Please let me know if further changes are required. |
| assert len(j["layers"][0]["features"]) == 1 | ||
| assert j["layers"][0]["features"][0]["fid"] == 0 | ||
|
|
||
| assert "fid" in j["layers"][0]["features"][0] |
There was a problem hiding this comment.
why this change ? The original test was more appropriate. We want to check that the returned feature has the fid we specified
There was a problem hiding this comment.
You’re right ,restoring the original assertion makes more sense since we want to verify that the returned feature matches the requested FID. I will be reverting to checking equality.
|
…h prev test and doc error resolved
| .SetRemoveSQLCommentsEnabled(); | ||
| AddArg("fid", 0, _("Feature identifier"), &m_fid) | ||
| .SetMetaVar("FID") | ||
| .SetMutualExclusionGroup("layer-sql") |
| } | ||
| if (m_fid >= 0) | ||
| { | ||
| aosOptions.AddString("-fid"); |
There was a problem hiding this comment.
wrong indentation...
sigh... please be respectful of maintainer time, and submit commits that build, and setup pre-commit as indicated in https://gdal.org/en/stable/development/dev_practices.html#commit-hooks
There was a problem hiding this comment.
Sorry for the formatting and build issues I’ve fixed them and will set up the pre-commit hooks and run first to prevent this in future.
2 participants
This plugs the --fid option of gdal vector info into the existing
-fid option handled by GDALVectorInfo(), without re-implementing
any logic at the algorithm layer.
fixes #13763