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

Documentation inconistency #532

Closed
3 of 4 tasks
blaginin opened this issue Feb 2, 2023 · 4 comments · Fixed by #565 or #600
Closed
3 of 4 tasks

Documentation inconistency #532

blaginin opened this issue Feb 2, 2023 · 4 comments · Fixed by #565 or #600
Labels
documentation Improvements or additions to documentation

Comments

@blaginin
Copy link
Collaborator

blaginin commented Feb 2, 2023

  • TIA Toolbox version: 1.3.1, dev
  • Python version: 3.10.8 (conda)
  • Operating System: MacOS 13.1 (22C65)

Description

Trying to learn the library and discovered some cases where docs are inconsistent with the current behaviour.

What I Did

>>>  store = AnnotationStore()
TypeError: Can't instantiate abstract class AnnotationStore with abstract methods __getitem__, __len__, __setitem__, commit, dump, dumps, open

AnnotationStore has become abstract, and it would be helpful to update the docs accordingly. Examples can either state that users must use derived classes or explicitly show methods usage on SQLiteStore or DictionaryStore.

>>> store = DictionaryStore()
>>> store.add(
...     Annotation(
...         geometry=Polygon.from_bounds(0, 0, 1, 1),
...         properties={"class": 42},
...     ),
...     key="foo",
... )
AttributeError: 'DictionaryStore' object has no attribute 'add'

It looks like .add.append?

  • Tried to query my AnnotationStore (as it was stated here and here).
>>> store = DictionaryStore()
>>> store.append(
...     Annotation(
...         geometry=Polygon.from_bounds(0, 0, 1, 1),
...         properties={"class": 42},
...     ),
...     key="foo",
... )
>>> store.bquery(where="props['class'] == 42") 
AttributeError: 'NoneType' object has no attribute 'bounds'

The method works successfully with SQLiteStore but fails on DictionaryStore. Maybe it would be helpful to add a test for that behaviour.

  • Tried to predict classes of batches (as it was stated here)
>>> wsi_data = wsi.slide_thumbnail(resolution=5, units="power")
>>> assert type(wsi_data) == np.ndarray

>>> predictor = PatchPredictor(pretrained_model="resnet18-kather100k", batch_size=32)
>>> tile_output = predictor.predict(
...    imgs=[wsi_data],
...    mode="tile",
...    merge_predictions=True,
...    patch_input_shape=[224, 224],
...    stride_shape=[224, 224],
...    resolution=1,
...    units="baseline",
...    return_probabilities=True,
...    on_gpu=False
... )
TypeError: expected str, bytes or os.PathLike object, not ndarray

The method works with filenames but not with ndarray objects. Thus, users have to save their intermediate images to call the methods, which seems like an extra step.

@shaneahmed
Copy link
Member

@blaginin Please create a PR to suggest edits to the documentation. You can also look at #540 if you have any suggestions.

@shaneahmed shaneahmed added the documentation Improvements or additions to documentation label Mar 3, 2023
@John-P John-P linked a pull request Mar 11, 2023 that will close this issue
@shaneahmed
Copy link
Member

Hi @blaginin, Do you have any comments on #565 ?

@blaginin
Copy link
Collaborator Author

Hi @blaginin, Do you have any comments on #565 ?

@shaneahmed, all fixed except the last point. I can change it in a separate PR.

@John-P
Copy link
Contributor

John-P commented Mar 15, 2023

👍 Yep, I think the last point should be address in a separate PR (and probably a separate issue to keep track and so this one can be closed).

@shaneahmed shaneahmed reopened this Mar 16, 2023
This was referenced May 5, 2023
shaneahmed added a commit that referenced this issue May 5, 2023
## 1.4.0 (2023-04-24)

### Major Updates and Feature Improvements

- Adds Python 3.11 support \[experimental\] #500
  - Python 3.11 is not fully supported by `pytorch` pytorch/pytorch#86566 and `openslide` openslide/openslide-python#188
- Removes Python 3.7 support
  - This allows upgrading all the dependencies which were dependent on an older version of Python.
- Adds Neighbourhood Querying Support To AnnotationStore #540
  - This enables easy and efficient querying of annotations within a neighbourhood of other annotations.
- Adds `MultiTaskSegmentor` engine #424
- Fixes an issue with stain augmentation to apply augmentation to only tissue regions.
  - #546 contributed by @navidstuv
- Filters logger output to stdout instead of stderr.
  - Fixes #255
- Allows import of some modules at higher level for improved usability
  - `WSIReader` can now be imported as `from tiatoolbox.wsicore import WSIReader`
  - `WSIMeta` can now be imported as `from tiatoolbox.wsicore import WSIMeta`
  - `HoVerNet`, `HoVerNetPlus`, `IDaRS`, `MapDe`, `MicroNet`, `NuClick`, `SCCNN` can now be imported as \`from tiatoolbox.models import HoVerNet, HoVerNetPlus, IDaRS, MapDe, MicroNet, NuClick, SCCNN
- Improves `PatchExtractor` performance. Updates `WSIPatchDataset` to be consistent. #571
- Updates documentation for `License` for clarity on source code and model weights license.

### Changes to API

- Updates SCCNN architecture to make it consistent with other models. #544

### Bug Fixes and Other Changes

- Fixes Parsing Missing Omero Version NGFF Metadata #568
  - Fixes #535 raised by @benkamphaus
- Fixes reading of DICOM WSIs at the correct level #564
  - Fixes #529
- Fixes `scipy`, `matplotlib`, `scikit-image` deprecated code
- Fixes breaking changes in `DICOMWSIReader` to make it compatible with latest `wsidicom` version. #539, #580
- Updates `shapely` dependency to version >=2.0.0 and fixes any breaking changes.
- Fixes bug with `DictionaryStore.bquery` and `geometry=None`, i.e. only a where predicate given.
  - Partly Fixes #532 raised by @blaginin
- Fixes local tests for Windows/Linux
- Fixes `flake8`, `deepsource` errors.
- Uses `logger` instead of `warnings` and `print` statements to properly log runs.

### Development related changes

- Upgrades dependencies which are dependent on Python 3.7
- Moves `requirements*.txt` files to `requirements` folder
- Removes `tox`
- Uses `pyproject.toml` for `bdist_wheel`, `pytest` and `isort`
- Adds `joblib` and `numba` as dependencies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
3 participants