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

Pip install #77

Closed
wants to merge 4 commits into from
Closed

Pip install #77

wants to merge 4 commits into from

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Dec 14, 2021

Add schemas.py from ome/ome-zarr-py#142 so that we can do:

Built and pushed to test-pypi for testing with

$ python setup.py sdist bdist_wheel
$ -m twine upload --repository testpypi dist/*

To test:

pip install -i https://test.pypi.org/simple/ ome-ngff==0.0.4.dev0

Then:

from ngff.schemas import LocalRefResolver, get_schema

version = "0.3"
strict_schema = get_schema(version, strict=True)

# Use our local resolver subclass to resolve local documents
localResolver = LocalRefResolver.from_schema(strict_schema)

NB: packaging included lots of files eg. 0.x/examples/ but I failed to exclude with a MANIFEST.in.


setup(
name="ngff",
version="0.0.4dev",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version probably expresses best my concerns about the multi-type repository. Turning it into a question, what is our positions on the different versions in this repository? I see at least 3 sources of truth:"

  • the version of the Python library shipping the schemas (0.0.4.dev)
  • the version of the latest schema (currently 0.3.0)
  • the latest repository tag (currently 0.1.3)
    Are these versions expected to be in sync or evolve independently. For all the versions that are in sync, can we use a single-source of truth.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "version of the latest schema" is defined by the most recent top-level directory, currently /0.3/ but I think that doesn't have to match the current version in setup.py, since the directory will never be named *dev. But at each release they should match and correspond to the repo tag (with bumpversion being the single source of truth).

setup.py Outdated
description="Next-Generation File Format: spec and schemas",
long_description=read("README.md"),
long_description_content_type="text/markdown",
packages=["ngff"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See ome/ome-zarr-py#142 (review) for a suggestion of prefixing the package with ome_

"Programming Language :: Python",
"Programming Language :: Python :: 3",
"Operating System :: OS Independent",
"License :: OSI Approved :: BSD License",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another issue to clarify is that the repository is currently marked under the W3C Sofware and Document License - see https://github.com/ome/ngff/blob/main/LICENSE.md

@sbesson
Copy link
Member

sbesson commented Jan 7, 2022

(base) sbesson@ls30630:tmp $ venv/bin/pip install -i https://test.pypi.org/simple/ ngff==0.0.4.dev0
Looking in indexes: https://test.pypi.org/simple/
Collecting ngff==0.0.4.dev0
  Downloading https://test-files.pythonhosted.org/packages/35/ac/1402a5c9be7e837fac5fb0db4cea57bb9fb89aa7a5319a0c6a7031cddca4/ngff-0.0.4.dev0-py3-none-any.whl (3.1 kB)
Requirement already satisfied: jsonschema in ./venv/lib/python3.8/site-packages (from ngff==0.0.4.dev0) (4.3.3)
Requirement already satisfied: attrs>=17.4.0 in ./venv/lib/python3.8/site-packages (from jsonschema->ngff==0.0.4.dev0) (21.4.0)
Requirement already satisfied: pyrsistent!=0.17.0,!=0.17.1,!=0.17.2,>=0.14.0 in ./venv/lib/python3.8/site-packages (from jsonschema->ngff==0.0.4.dev0) (0.18.0)
Requirement already satisfied: importlib-resources>=1.4.0 in ./venv/lib/python3.8/site-packages (from jsonschema->ngff==0.0.4.dev0) (5.4.0)
Requirement already satisfied: zipp>=3.1.0 in ./venv/lib/python3.8/site-packages (from importlib-resources>=1.4.0->jsonschema->ngff==0.0.4.dev0) (3.7.0)
Installing collected packages: ngff
Successfully installed ngff-0.0.4.dev0
(base) sbesson@ls30630:tmp $ venv/bin/python 
Python 3.8.5 (default, Sep  4 2020, 02:22:02) 
[Clang 10.0.0 ] :: Anaconda, Inc. on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from ngff.schemas import LocalRefResolver, get_schema
>>> version = "0.3"
>>> strict_schema = get_schema(version, strict=True)
curr_dir /private/tmp/venv/lib/python3.8/site-packages/ngff
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/private/tmp/venv/lib/python3.8/site-packages/ngff/schemas.py", line 39, in get_schema
    return load_json(path)
  File "/private/tmp/venv/lib/python3.8/site-packages/ngff/schemas.py", line 25, in load_json
    with open(path) as f:
FileNotFoundError: [Errno 2] No such file or directory: '/private/tmp/venv/lib/python3.8/site-packages/0.3/schemas/strict_image.schema'

so at least locally it looks like the schemas are not bundled

On the exclusion of examples, probably also a topic up for discussion but in the case of ome-model, we had several conversation about the potential value of packaging our XML samples in the specification JAR.

@will-moore
Copy link
Member Author

Trying to get MANIFEST.in to include 0.N/schemas
With NO MANIFEST.in
I get everything in /0.1 and /0.2 and /0.3 included. e.g.

Also get everything with:

recursive-include "0.2" *
$ python setup.py sdist bdist_wheel
...
copying 0.3/.tox/v03/log/v03-0.log

recursive-include 0.2/schemas * seems to copy everything under 0.2/ and 0.3/ but not seen copying 0.1/, but it is there in the dist/ome-ngff-0.0.4.dev0.tar.gz.

With this (or empty MANIFEST.in) I get everything copied

recursive-exclude examples
global-include image.schema

With this, I get warning: no previously-included files matching '*' found under directory 'examples' but don't get any of the 0.1/ or 0.2/ or 0.3/ directory contents, unless the file ends with a blank line!
EDIT - tried again later and I got everything!

recursive-exclude examples *
global-include image.schema

Switching them the other way up gives me everything if the file ends with a blank line!

global-include image.schema
recursive-exclude examples *

Always making sure to not include a blank line now...

This still gives me everything:

recusive-include 0.1 *
recursive-exclude examples *

and so does this, maybe because 0.1 is taken as a regex that matches everything? No...

recusive-include 0.1
recursive-exclude examples *

Ooops - just noticed a typo in recusive in last 2 examples!? So don't understand why they're copying everything.
This also includes everything:

recursive-include latest *
recursive-exclude examples *

So does:

recursive-include latest *

and this also gives everything:

recursive-exclude examples *
recursive-include latest *

and so does:

recursive-exclude examples *
recursive-include latest

This includes everything:

recursive-exclude examples *
graft latest

Can't get a handle on this - even getting different results with the same MANIFEST.in at different times... 😢

@constantinpape
Copy link
Contributor

I am not quite sure what's blocking here. Is it just the question about how to include the schemas into the package data?

@sbesson
Copy link
Member

sbesson commented Mar 22, 2022

The first issue is functional I did not manage to get this working in #77 (comment). It might be ready to be retested with the last commit?

Additionally, I am still unsure of how much Python specificity should be introduced into this repository. We have a very similar experience in https://github.com/ome/ome-model which includes some specification (OME XSD schemas), the OME-XML Java library and formerly the OME-XML C++ library. When working actively on the specification, this level of high coupling makes complete sense as you want these components to be updated and maintained in sync. As the library matures, this tends to create unnecessary coupling i.e. a fix that only affects the Python package would requires a release of any othe component. #77 (comment) is a concrete instance of a lifecycle question that is generated by this coupling.

@will-moore
Copy link
Member Author

I couldn't work out how to get the schemas included, but not to include everything under /0.2/*, /0.3/*, /0.4/* including all the .tox/ files etc.

@constantinpape
Copy link
Contributor

Additionally, I am still unsure of how much Python specificity should be introduced into this repository. We have a very similar experience in https://github.com/ome/ome-model which includes some specification (OME XSD schemas), the OME-XML Java library and formerly the OME-XML C++ library. When working actively on the specification, this level of high coupling makes complete sense as you want these components to be updated and maintained in sync. As the library matures, this tends to create unnecessary coupling i.e. a fix that only affects the Python package would requires a release of any othe component. #77 (comment) is a concrete instance of a lifecycle question that is generated by this coupling.

My 2cents: I would avoid doing anything python specific for distributing the spec and rather implement this in ome-zarr-py.
(e.g. download to some library dir via requests). I think that this is a simpler and cleaner solution.

I couldn't work out how to get the schemas included, but not to include everything under /0.2/*, /0.3/*, /0.4/* including all the .tox/ files etc.

Yes, somehow including non-python files with setuptools is a huge pain...

@joshmoore
Copy link
Member

👍 for a download based workflow in the language-specific implementations. Perhaps the ngff build creates a bundle that can be accessed e.g. ngff.openmicroscopy.org/$version/bundle.tgz

@will-moore
Copy link
Member Author

Would it be possible to update the schemas in the ngff repo, and run the ome_zarr validate commands to consume those at the same time? The pip install workflow allows this, but maybe that's setting the bar too high?

@sbesson
Copy link
Member

sbesson commented Mar 24, 2022

Would it be possible to update the schemas in the ngff repo, and run the ome_zarr validate commands to consume those at the same time? The pip install workflow allows this, but maybe that's setting the bar too high?

I am not sure I understand. Are you suggesting that ome_zarr validate should depend at runtime on schemas hosted on ngff.openmicroscopy.org ? Or that the installation of ome-zarr should include some mechanism downloading these schemas?

@will-moore
Copy link
Member Author

I think, when I was working on ome/ome-zarr-py#142, I was able to pip install -e . from the ngff repo, then edit the schemas in the ngff repo and test them with ome_zarr validate at the same time (no build, release, deploy steps).
But maybe that's an edge case and we don't really need to support it.

@joshmoore
Copy link
Member

I was assuming the download would be packaged into the pip installable.

@will-moore
Copy link
Member Author

The first issue is functional I did not manage to get this working in #77 (comment). It might be ready to be retested with the last commit?

No, I don't think it's worth re-testing. I didn't make any progress on packaging above. Not sure what to try next.
If that's the only blocker, I can try looking at it some more?
And also remove the python code get_schema() etc?

@will-moore
Copy link
Member Author

Going with a different strategy at ome/ome-zarr-py#142

@will-moore will-moore closed this Jul 5, 2022
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.

4 participants