FIX documentation, packaging and API mismatches - #53
Merged
aradfarahani merged 1 commit intoAug 21, 2026
Merged
Conversation
Addresses the seven items in the docs/API issue. Two are packaging bugs rather than documentation errors. Packaging. fezrs/tools/change_detection/ and fezrs/tools/svm/ had no __init__.py, so nothing could be imported from either subpackage, and find_packages(include=["fezrs", "fezrs.*"]) in setup.py did not collect them at all. fezrs/media/ had the same problem while holding the watermark asset. fezrs/tools/mosaic/__init__.py re-exported BaseTool instead of MosaicCalculator, so fezrs.tools.mosaic.MosaicCalculator did not exist while fezrs.tools.mosaic.BaseTool did. All four are fixed. Documentation. fezrs.tools.indices and fezrs.tools.enhancement do not exist - the packages are spectral_indices and image_enhancement - and GuassianCalculator was a typo. With the packaging fixes, all 30 import statements appearing in docs/, README.md and paper/ now execute. README. chart_export() does not exist and never did; the method is histogram_export(), so the first thing a new user copied raised AttributeError. The module list named 28 of 37 exported calculators, omitting the entire change-detection group, SVM, mosaic and the import tools. The FEZtool link fails to load because feztool.com currently serves an expired TLS certificate; it now points at the GitHub organisation. paper.md carried the same chart_export reference. base.py. filename_prefix was accepted by every calculator, forwarded correctly, and then overwritten with the tool name on the first line of _export_file() before it was ever used. It now defaults to None and falls back to the tool name, so default filenames are unchanged while an explicit prefix is honoured. nrows and ncols were declared and documented on execute() but never reached _export_file(); they are now forwarded, and added to the signatures of the 15 calculators that lacked them, so the same keyword no longer raises TypeError on some tools while being silently ignored on others. Examples. All four scripts resolve paths from Path(__file__).parent and run from any working directory. Previously three required being run from example/ and one from the repository root. PCA. selectBand indexed into a fixed band-name to component-index map, so selectBand="red" plotted the first principal component and titled the figure "Histogram of PCA Band Red", attributing the output to an input band that did not produce it - a principal component is a linear combination of all six bands. Replaced with component (1..6); selectBand still works and warns, naming the component it resolves to. Expose explained_variance_ratio_ and components_. The loadings matter more than the variance share: a target is isolated by the component whose diagnostic bands carry high loadings of opposing sign, which is often not the leading component. Fix the sign indeterminacy so repeated runs and crops of the same scene are comparable, and add standardize= for correlation-matrix PCA, since sklearn's covariance decomposition lets the widest-range band dominate regardless of information content. Add Landsat8Calculator and GeoeyeCalculator aliases, keeping the underscored names exported.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #52 — review #50, #51, #52 first; this PR targets
hotfix/spectral-index-radiometry.All seven items from the docs/API issue. Two turned out to be packaging bugs rather than documentation errors.
1. Packaging — two subpackages were unimportable
fezrs/tools/change_detection/andfezrs/tools/svm/had no__init__.py. Nothing could be imported from either, andfind_packages(include=["fezrs", "fezrs.*"])insetup.pydid not collect them at all.fezrs/media/had the same problem, while holding the watermark the base class loads.fezrs/tools/mosaic/__init__.pyre-exportedBaseToolinstead ofMosaicCalculator, sofezrs.tools.mosaic.MosaicCalculatordid not exist whilefezrs.tools.mosaic.BaseTooldid.A test now asserts
find_packagescollects all four.2. Documented imports
fezrs.tools.indices→spectral_indices,fezrs.tools.enhancement→image_enhancement,GuassianCalculator→GaussianCalculator. Combined with the packaging fixes, all 30 import statements indocs/,README.mdandpaper/now execute — verified by a new parametrised test that extracts and runs each one, so this cannot drift again.3. README
chart_export()→histogram_export(). The method never existed, so the first thing a new user copied raisedAttributeError.feztool.comcurrently serves an expired TLS certificate, which is why it fails to load. The domain resolves fine. I pointed the link at the GitHub organisation for now, but the real fix is renewing the certificate, after which you may want the original link back.paper.mdcarried the samechart_exportreference.4.
base.py— three arguments accepted and then ignoredfilename_prefixwas accepted by all 37 calculators, forwarded correctly, and then overwritten with the tool name on the first line of_export_file()before it was used. It now defaults toNoneand falls back to the tool name, so default filenames are unchanged while an explicit prefix is honoured.nrows/ncolswere declared and documented onexecute()but never reached_export_file(). Now forwarded, and added to the 15 calculator signatures that lacked them — previously the same keyword raisedTypeErroron the index tools while being silently ignored on the enhancement tools.5. Examples
All four resolve paths from
Path(__file__).parentand run from any working directory. Previously three requiredexample/as cwd and one required the repo root.6. PCA — the one public API change
selectBandindexed into a fixed band-name → component-index map, soselectBand="red"plotted the first principal component and titled the figure "Histogram of PCA Band Red". A principal component is a linear combination of all six bands weighted by its eigenvector, so no component corresponds to an input band — the label attributed the output to a band that did not produce it, and someone selecting"swir2"to inspect the SWIR2 response was shown the fifth component. The map order was arbitrary too, following the insertion order of the band dict inFileHandler.__init__.Replaced with
component(1–6).selectBandstill works and warns, naming the component it resolves to.Three additions while in there, all aimed at making the module actually usable for the analysis it is meant for:
explained_variance_ratio_andcomponents_exposed.process()already computed both and stored the estimator onself._pca, with no public accessor. The loadings matter more than the variance share: a target is isolated by the component whose diagnostic bands carry high loadings of opposing sign, and that is frequently not the leading component — PC1 usually just encodes scene brightness. On the bundled data PC1 carries 90.6%, PC2 7.3%, PC3 1.8%.standardize=added for correlation-matrix PCA.sklearndecomposes the covariance matrix, which lets whichever band has the widest DN range dominate the leading components regardless of information content. Defaults toFalse, preserving current behaviour.7. Naming
Landsat8CalculatorandGeoeyeCalculatoraliases added; the underscored names stay exported.Tests
393 pass (347 on #52, 46 new):
fezrs.__all__; subpackage importability;find_packagescoverage.filename_prefixhonoured and still defaulting to the tool name;nrows/ncolsreaching_export_file.selectBanddeprecation naming its component, mutual exclusion, both accessors raising beforeprocess(), sign determinism across runs, andstandardizechanging the variance structure.12 existing test modules had their
assert_called_once_withexpectations updated for the newexecute()signature. All four examples verified running from an unrelated working directory.