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

added pyproject.toml file #158

Merged
merged 6 commits into from
Jul 23, 2023
Merged

Conversation

swaradgat19
Copy link
Contributor

@swaradgat19 swaradgat19 commented Jul 20, 2023

fixes #134

@kaczmarj
Copy link
Member

kaczmarj commented Jul 20, 2023

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@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
Copy link
Member

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

@swaradgat19
Copy link
Contributor Author

swaradgat19 commented Jul 21, 2023

@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
Copy link
Member

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

@swaradgat19
Copy link
Contributor Author

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

@swaradgat19
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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

@kaczmarj
Copy link
Member

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
Copy link
Contributor Author

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

@kaczmarj kaczmarj merged commit 147f17e into SBU-BMI:main Jul 23, 2023
@swaradgat19 swaradgat19 deleted the fix/pyproject.toml branch August 11, 2023 21:09
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.

use pyproject.toml instead of setup.cfg
2 participants