Skip to content

FIX documentation, packaging and API mismatches - #53

Merged
aradfarahani merged 1 commit into
hotfix/spectral-index-radiometryfrom
hotfix/docs-api-mismatches
Aug 21, 2026
Merged

FIX documentation, packaging and API mismatches#53
aradfarahani merged 1 commit into
hotfix/spectral-index-radiometryfrom
hotfix/docs-api-mismatches

Conversation

@mkiani12

Copy link
Copy Markdown
Collaborator

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/ and fezrs/tools/svm/ had no __init__.py. Nothing could be imported from either, 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 the base class loads.
  • 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.

A test now asserts find_packages collects all four.

2. Documented imports

fezrs.tools.indicesspectral_indices, fezrs.tools.enhancementimage_enhancement, GuassianCalculatorGaussianCalculator. Combined with the packaging fixes, all 30 import statements in docs/, README.md and paper/ 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 raised AttributeError.
  • Module list went from 28 to all 37 exported calculators — it had omitted the entire change-detection group, SVM, mosaic and the import tools.
  • The FEZtool link: worth knowing the specific cause — feztool.com currently 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.md carried the same chart_export reference.

4. base.py — three arguments accepted and then ignored

filename_prefix was 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 to None and falls back to the tool name, so default filenames are unchanged while an explicit prefix is honoured.

nrows/ncols were declared and documented on execute() but never reached _export_file(). Now forwarded, and added to the 15 calculator signatures that lacked them — previously the same keyword raised TypeError on the index tools while being silently ignored on the enhancement tools.

5. Examples

All four resolve paths from Path(__file__).parent and run from any working directory. Previously three required example/ as cwd and one required the repo root.

6. PCA — the one public API change

selectBand indexed into a fixed band-name → component-index map, so selectBand="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 in FileHandler.__init__.

Replaced with component (1–6). selectBand still 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_ and components_ exposed. process() already computed both and stored the estimator on self._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%.
  • Component signs fixed. Eigenvector signs are mathematically arbitrary, so the same scene could yield an inverted component image between runs or between a scene and a crop — the same class of reproducibility defect as Spectral indices are computed on per-band min–max normalized data, making them scene-dependent #37. The largest-magnitude loading of each component is now forced positive.
  • standardize= added for correlation-matrix PCA. sklearn decomposes the covariance matrix, which lets whichever band has the widest DN range dominate the leading components regardless of information content. Defaults to False, preserving current behaviour.

7. Naming

Landsat8Calculator and GeoeyeCalculator aliases added; the underscored names stay exported.

Tests

393 pass (347 on #52, 46 new):

  • Every documented import executed, parametrised per statement.
  • README completeness vs fezrs.__all__; subpackage importability; find_packages coverage.
  • filename_prefix honoured and still defaulting to the tool name; nrows/ncols reaching _export_file.
  • PCA component selection and range validation, selectBand deprecation naming its component, mutual exclusion, both accessors raising before process(), sign determinism across runs, and standardize changing the variance structure.

12 existing test modules had their assert_called_once_with expectations updated for the new execute() signature. All four examples verified running from an unrelated working directory.

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.
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.

2 participants