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

Add docker build #14

Merged
merged 6 commits into from
Nov 13, 2020
Merged

Add docker build #14

merged 6 commits into from
Nov 13, 2020

Conversation

joshmoore
Copy link
Member

I recently ran into an issue opening n5 from zarr. I figured this repo was a good place to start to figure out what's going on. I'll see if I can make a change that makes that test fail. In the meantime, this is the setup I used to build locally. Note that there are a few failures:

  /opt/conda/envs/z/lib/python3.7/site-packages/pyn5/file_group.py:228: UserWarning: Expected N5 version '2.0.2', got 2.1.3; trying to open anyway
...
FAILED test/test_read_all.py::test_correct_read[read z5py zarr using zarr, gzip] - OSError: Not a gzipped file (b'x^')
FAILED test/test_read_all.py::test_correct_read[read zarr N5 using z5py, gzip] - RuntimeError: Exception during zlib decompression: ...
FAILED test/test_read_all.py::test_correct_read[read zarr N5 using z5py, raw] - assert False

@joshmoore
Copy link
Member Author

I found my issue. The sub-attributes.json files are missing "n5": "2.0.0". Moving this back to tischi/i2k-2020-s3-zarr-workshop#1

environment.yml Outdated
Comment on lines 2 to 6
channels:
- defaults
- ome
- conda-forge
- anaconda
Copy link
Member

Choose a reason for hiding this comment

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

anaconda is a weird metachannel, which isn't really intended for public use. conda-forge should be higher priority than defaults as that is how conda-forge packages are built and intended to be used.

Suggested change
channels:
- defaults
- ome
- conda-forge
- anaconda
channels:
- ome
- conda-forge
- defaults

environment.yml Outdated
Comment on lines 29 to 31
- pip:
- pre-commit
- pytest-qt
Copy link
Member

Choose a reason for hiding this comment

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

conda-forge packages of both of these exist. That said, do we need pre-commit? Is this environment intended for development or just testing?

Suggested change
- pip:
- pre-commit
- pytest-qt
- pre-commit
- pytest-qt

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry. This is a copy of the ome-zarr-py dev environment. Thanks for catching it.

@constantinpape
Copy link
Collaborator

Thanks @joshmoore. I think the failures you see are because I haven't merged #7 yet.
I will try to look into this soon and then also look at this PR, but let's focus on the issue with the missing attribute first.

@joshmoore
Copy link
Member Author

Updated based on @jakirkham's feedback and added a github workflow.

@joshmoore
Copy link
Member Author

Doh. I hadn't seen #12 -- Happy to rollback those changes, but https://github.com/constantinpape/zarr_implementations/actions/runs/361203107 now represents what I'm seeing locally.

@constantinpape
Copy link
Collaborator

@joshmoore this looks good. I am merging and deprecating #12.
I will try to fix the test failures in a separate PR.

@constantinpape constantinpape merged commit 1f5c1aa into zarr-developers:master Nov 13, 2020
@constantinpape constantinpape mentioned this pull request Nov 13, 2020
@joshmoore joshmoore deleted the docker branch November 16, 2020 08:27
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.

3 participants