SBU-BMI / wsinfer

🔥 🚀 Blazingly fast pipeline for patch-based classification in whole slide images
https://wsinfer.readthedocs.io
Apache License 2.0
56 stars 9 forks source link

added pyproject.toml file #158

Closed swaradgat19 closed 1 year ago

swaradgat19 commented 1 year ago

fixes #134

kaczmarj commented 1 year ago

try to install the package in a local development environment before your next push to this branch. there are some errors and warnings:

              ********************************************************************************
              The license_file parameter is deprecated, use license_files instead.

              By 2023-Oct-30, you need to update your project and remove deprecated calls
              or your builds will no longer be supported.

              See https://setuptools.pypa.io/en/latest/userguide/declarative_config.html for details.
              ********************************************************************************

and

      ValueError: invalid pyproject.toml config: `project`.
      configuration error: `project` must contain ['version'] properties

you can remove setup.cfg, setup.py, versioneer.py, and wsinfer/_version.py. please update wsinfer/__init__.py with the version like so (from here):

from importlib.metadata import version, PackageNotFoundError

try:
    __version__ = version("package-name")
except PackageNotFoundError:
    # package is not installed
    __version__ = "unknown version, package not installed"

before pushing to your branch, format your code with isort and black like this:

isort wsinfer
black wsinfer
kaczmarj commented 1 year ago

by the way, github has a nice feature where you can link issues and pull requests in comments. you can even indicate that a pull request will close an issue, and once the pull request is merged, the issue is automatically close. see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

i edited your first comment to say fixes #158 to link to that issue and close it automatically once this is merged.

swaradgat19 commented 1 year ago

try to install the package in a local development environment before your next push to this branch. there are some errors and warnings:

              ********************************************************************************
              The license_file parameter is deprecated, use license_files instead.

              By 2023-Oct-30, you need to update your project and remove deprecated calls
              or your builds will no longer be supported.

              See https://setuptools.pypa.io/en/latest/userguide/declarative_config.html for details.
              ********************************************************************************

and

      ValueError: invalid pyproject.toml config: `project`.
      configuration error: `project` must contain ['version'] properties

you can remove setup.cfg, setup.py, versioneer.py, and wsinfer/_version.py. please update wsinfer/__init__.py with the version like so (from here):

from importlib.metadata import version, PackageNotFoundError

try:
    __version__ = version("package-name")
except PackageNotFoundError:
    # package is not installed
    __version__ = "unknown version, package not installed"

before pushing to your branch, format your code with isort and black like this:

isort wsinfer
black wsinfer

Sure. Since we're deleting _version.py, we will be fetching the version in init.py itself?

swaradgat19 commented 1 year ago

@kaczmarj Solved the second error. Used the command python3 -m build in a new virtual environment. That gave me a tar.gz and .whl file. I installed the tar.gz file. Installation was successful, but wsinfer version comes out to be 0.0.0. Will look into it

kaczmarj commented 1 year ago

try install the .whl file instead. but i would hope the tar.gz also has the correct version...

swaradgat19 commented 1 year ago

@kaczmarj -

[tool.versioneer]
VCS = "git"
style = "pep440"
versionfile_source = "wsinfer/__init__.py"
versionfile_build = "wsinfer/__init__.py"
tag_prefix = "v"
parentdir_prefix = "wsinfer"

This is the tool.versioneer section in the toml file.


"""WSInfer is a toolkit for fast patch-based inference on whole slide images."""

from __future__ import annotations

from importlib.metadata import PackageNotFoundError
from importlib.metadata import version

from .modellib.run_inference import WholeSlideImagePatches  # noqa
from .modellib.run_inference import run_inference  # noqa

try:
    __version__ = version("wsinfer")
except PackageNotFoundError:
    # package is not installed
    __version__ = "unknown version, package not installed"

# Patch Zarr. See:
# https://github.com/bayer-science-for-a-better-life/tiffslide/issues/72#issuecomment-1627918238
# https://github.com/zarr-developers/zarr-python/pull/1454
def _patch_zarr_kvstore():
    from zarr.storage import KVStore

    def _zarr_KVStore___contains__(self, key):
        return key in self._mutable_mapping

    KVStore.__contains__ = _zarr_KVStore___contains__

_patch_zarr_kvstore()

This is the init.py file.

Even the whl file is named wsinfer-0.0.0-py3-none-any.whl. Could you see why this is going wrong?

kaczmarj commented 1 year ago

i'm not certain why but try removing versioneer entirely. remove the versioneer config from pyproject.toml as well.

swaradgat19 commented 1 year ago

My bad. I used [tool.setuptools_scm] instead and it installed successfully. I'll commit to my branch

swaradgat19 commented 1 year ago

I also wanted to ask. In the pyproject.toml file, I've included this line and it works just fine.

[tool.setuptools_scm]

Do we need additional arguments, like:

# pyproject.toml
[tool.setuptools_scm]
version_file = "wsinfer/__init__.py"

I'm not sure I understand the reason we're setting __version__ in __init__.py Is setuptools_scm fetching from init by default?

kaczmarj commented 1 year ago

looking good! can you also delete the file wsinfer/_version.py ? I don't think we need that. but do tell me if I'm wrong...

Do we need additional arguments, like:

I am not sure... I will look online and get back to you.

I'm not sure I understand the reason we're setting __version__ in __init__.py Is setuptools_scm fetching from init by default?

This is a convention in python packages. See PEP 396 for more specifics, but in general, packages are expected to define the version in a top-level __version__ variable. For example, try using numpy.__version__ or pandas.__version__ and you will see the installed version be printed.

so setuptools is not reading the __version__ variable in __init__. instead, setuptools is helping us to define that variable by using git tags and commit information (the version is the latest git tag plus any commits made after that tag).

swaradgat19 commented 1 year ago

Got it. Thanks for the explanation! 9 tests have passed. The toml should be good to go, mostly.

kaczmarj commented 1 year ago

thanks @swaradgat19 !

i've been reading into setuptools_scm. i think it would be best to write the version to a static file and read it from there. to do this, can you change the

in pyproject.toml, please add:

[tool.setuptools_scm]
write_to = "wsinfer/_version.py"

in __init__.py, please add:

try:
    from ._version import __version__
except ImportError:
    __version__ = "0.0.unknown"

and please add /wsinfer/_version.py to .gitignore

remember to run isort wsinfer and black wsinfer before committing. thanks for all your help on this! and thanks for getting this running so fast!

swaradgat19 commented 1 year ago

@kaczmarj My pleasure! Made the changes. Made separate commit for isort and black commands