Feat/qupath

[swaradgat19]

No description provided.

[swaradgat19]

@kaczmarj I believe the pytorch test is failing because I'm importing from paquo.projects import QuPathProject inside the make_qupath_project function and it does not identify QuPathProject in the add_image_and_geojson function.

[kaczmarj]

looks great @swaradgat19 ! i left a few requests for changes but overall, really great. thanks

[kaczmarj]

flake8 is complaining:

 wsinfer/qupath.py:9:18: F821 undefined name 'QuPathProject'

[swaradgat19]

flake8 is complaining:

 wsinfer/qupath.py:9:18: F821 undefined name 'QuPathProject'

@kaczmarj Instead of type QuPathProject in the add_image_and_geojson function, we can use object type like this:

def add_image_and_geojson(
    qupath_proj: object,
    *,
    image_path: Path | str,
    geojson_path: Path | str,
) -> None:
# rest of the code

It is not recognizing QuPathProject inside add_image_and_geojson since we're importing QuPathProject (basically paquo) in another function (make_qupath_project)

Do you think this would be a good idea?

[kaczmarj]

@kaczmarj Instead of type QuPathProject in the add_image_and_geojson function, we can use object type like this:

def add_image_and_geojson(
    qupath_proj: object,
    *,
    image_path: Path | str,
    geojson_path: Path | str,
) -> None:
# rest of the code

It is not recognizing QuPathProject inside add_image_and_geojson since we're importing QuPathProject (basically paquo) in another function (make_qupath_project)

Do you think this would be a good idea?

i suggest we remove the type, so it's def add_image_and_geojson(qupath_proj, ...)

[swaradgat19]

@kaczmarj I believe not specifying a type is causing an issue:

wsinfer/qupath.py:8: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]

This was in the styles ci test

[kaczmarj]

kaczmarj I believe not specifying a type is causing an issue:

wsinfer/qupath.py:8: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]

This was in the styles ci test

i see... so instead of importing paquo inside of the function, let's import it globally inside of a try/except block. something like this...

try:
    from paquo.projects import QuPathProject
    HAS_PAQUO = True
except Exception:
    HAS_PAQUO = False   

and in the command-line interface, if the user passes --qupath, let's check that HAS_PAQUO is True. if it's not, we should error before any processing is done.

[swaradgat19]

@kaczmarj

Sure. So I'm setting the HAS_PAQUO variable globally like you suggested. Inside the make_qupath_project function, I'm doing the following:

def make_qupath_project(wsi_dir: Path, results_dir: Path) -> None:
    if not HAS_PAQUO:
        print(
            """Cannot find QuPath.
QuPath is required to use this functionality but it cannot be found.
If QuPath is installed, please use define the environment variable
PAQUO_QUPATH_DIR with the location of the QuPath installation.
If QuPath is not installed, please install it from https://qupath.github.io/."""
        )
        sys.exit(1)
    else:
        print("Found QuPath successfully!")
        QUPATH_PROJECT_DIRECTORY = results_dir / "model-outputs-qupath"
        # add image and geojson using paquo

[kaczmarj]

that looks good. feel free to push a commit with the changes

[swaradgat19]

I think its failing because even though the paquo import is global, I'm specifically calling the make_qupath_project function in the infer.py file, which is why it is not recognizing the global paquo import. I'm thinking of handling the import in the infer.py file itself.

For example:

    if qupath:
        #handle paquo import using try-except and if successful, call make_qupath_project function
       make_qupath_project(wsi_dir, results_dir)

[kaczmarj]

here are the errors.

we will have to add paquo.projects to mypy's ignored list in pyproject.toml. i guess paquo doesn't ship with types.

the other type errors seem to be new and i think unrelated to this pull request. let me work on fixing those in a separate branch. once i make the fix and i merge it into main, i will ask you to merge main into your branch.

wsinfer/qupath.py:8: error: Cannot find implementation or library stub for module named "paquo.projects"  [import-not-found]
wsinfer/qupath.py:8: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
wsinfer/patchlib/patch.py:56: error: Incompatible types in assignment (expression has type "Mat | ndarray[Any, dtype[generic]]", variable has type "ndarray[Any, dtype[signedinteger[Any]]]")  [assignment]
wsinfer/patchlib/patch.py:68: error: Unsupported operand types for * ("ndarray[Any, dtype[generic]]" and "ndarray[Any, dtype[floating[Any]]]")  [operator]
wsinfer/patchlib/patch.py:118: error: Incompatible return value type (got "tuple[Any, Sequence[ndarray[Any, dtype[generic]]], ndarray[Any, dtype[signedinteger[Any]]]]", expected "tuple[Any, Sequence[ndarray[Any, dtype[signedinteger[Any]]]], ndarray[Any, dtype[signedinteger[Any]]]]")  [return-value]
wsinfer/patchlib/segment.py:50: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[generic]]", variable has type "ndarray[Any, dtype[unsignedinteger[_8Bit]]]")  [assignment]
wsinfer/patchlib/segment.py:63: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[generic]]", variable has type "ndarray[Any, dtype[unsignedinteger[_8Bit]]]")  [assignment]
wsinfer/patchlib/segment.py:66: error: Incompatible types in assignment (expression has type "Mat | ndarray[Any, dtype[generic]]", variable has type "ndarray[Any, dtype[unsignedinteger[_8Bit]]]")  [assignment]

[kaczmarj]

@swaradgat19 - i merged main into your branch

[kaczmarj]

now the remaining errors are

wsinfer/qupath.py:8: error: Cannot find implementation or library stub for module named "paquo.projects"  [import-not-found]
wsinfer/qupath.py:8: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

to fix that, look at pyproject.toml. there should be a section where we tell mypy what to ignore. add paquo.* there

[swaradgat19]

@kaczmarj

I'm currently ignoring the qupath_proj type. I tried setting the type to object but that did not work. I just used # type: ignore. Still waiting for it to pass.

Sorry for the unnecessary commits!

[kaczmarj]

make

    try:
        from paquo.projects import QuPathProject

        HAS_PAQUO = True
    except Exception:
        HAS_PAQUO = False

top-level in qupath.py. (in other words, take it out of the function)

then, use QuPathProject as the type for qupath_proj.

also remove the type: ignore comment on the same line as qupath_proj

[swaradgat19]

@kaczmarj

My bad. I changed it because a global import wasn't working and I was running into issues. The issue was somewhere else. Changing it.

[swaradgat19]

Looks good! Should I work on the tests for this feature as well?

[kaczmarj]

great! i merged it into main. thanks very much @swaradgat19 !

Looks good! Should I work on the tests for this feature as well?

yes, that would be great

[swaradgat19]

So I'm thinking of modifying the test_cli_run_with_registered_models test with additional the qupath argument. Should I instead make a new test?

[kaczmarj]

I would make a dedicated new test. You can also skip the test is HAS_PAQUO is false. Pytest has a skipif decorator for this. Best,JakubOn Jan 5, 2024, at 17:30, Swarad Gat ***@***.***> wrote: So I'm thinking of modifying the test_cli_run_with_registered_models test with additional the qupath argument. Should I instead make a new test? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you modified the open/close state.Message ID: ***@***.***>