cogeotiff / rio-cogeo

Cloud Optimized GeoTIFF creation and validation plugin for rasterio
https://cogeotiff.github.io/rio-cogeo/
BSD 3-Clause "New" or "Revised" License
315 stars 42 forks source link

New version of click (8.x.x) may have been causing an issue with cog_translate #203

Closed ghelobytes closed 3 years ago

ghelobytes commented 3 years ago

To re-create, run test:

$ pytest -k test_cog_translate_valid
rio_cogeo/cogeo.py:289: in cog_translate
    with click.progressbar(wind, file=fout, show_percent=True) as windows:  # type: ignore
.direnv/python-3.7.3/lib/python3.7/site-packages/click/_termui_impl.py:98: in __enter__
    self.render_progress()
.direnv/python-3.7.3/lib/python3.7/site-packages/click/_termui_impl.py:220: in render_progress
    echo(self.label, file=self.file, color=self.color)
.
.
.
>       file.write(out)  # type: ignore
E       AttributeError: 'str' object has no attribute 'write'

.direnv/python-3.7.3/lib/python3.7/site-packages/click/utils.py:298: AttributeError

Locking down to click==7.1.2 seems to work around the issue:

$ pip install click==7.1.2
$ pytest -k test_cog_translate_valid
=============================================================================== test session starts ===============================================================================
platform darwin -- Python 3.7.3, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /Users/gcihj/Projects/_tools/rio-cogeo
plugins: cov-2.12.1
collected 74 items / 67 deselected / 7 selected

tests/test_cogeo.py .......                                                                                                                                                 [100%]

================================================================================ warnings summary =================================================================================
../../../.pyenv/versions/3.7.3/lib/python3.7/importlib/_bootstrap.py:219
  /Users/gcihj/.pyenv/versions/3.7.3/lib/python3.7/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject
    return f(*args, **kwds)

rio_cogeo/profiles.py:184
  /Users/gcihj/Projects/_tools/rio-cogeo/rio_cogeo/profiles.py:184: UserWarning: Non-standard compression schema: webp. The output COG might not be fully supported by software not build against latest libtiff.
    " supported by software not build against latest libtiff.".format(key)

-- Docs: https://docs.pytest.org/en/stable/warnings.html
================================================================== 7 passed, 67 deselected, 2 warnings in 1.66s =====

Maybe a simple fix for now is to modify requirements.txt: https://github.com/cogeotiff/rio-cogeo/blob/b1c3692b5adaa456dbc565860e9fa26453c5ad10/requirements.txt#L1-L5

vincentsarago commented 3 years ago

thanks @ghelobytes for the report, my co-worked just mentioned this to me and it all comes down to the latest rasterio update ref: https://github.com/mapbox/rasterio/pull/2191

I'll try to fix this tomorrow

vincentsarago commented 3 years ago

@ghelobytes I'm not able to reproduce the issue. can you share the click and rasterio version that you have?

ghelobytes commented 3 years ago

@vincentsarago

>>> import rio_cogeo, click, rasterio
>>> print(rio_cogeo.__version__)
2.2.2
>>> print(click.__version__)
8.0.1
>>> print(rasterio.__version__)
1.2.4
>>>
ghelobytes commented 3 years ago

From the latest click doc, it looks like it's now expecting a stream (TextIO):

file (Optional[TextIO]) – The file to write to. If this is not a terminal then only the label is printed.

This seems to work when I change the line below to this: fout = open(os.devnull, 'w') if quiet else sys.stderr but not sure if that's the appropriate thing to do. https://github.com/cogeotiff/rio-cogeo/blob/b1c3692b5adaa456dbc565860e9fa26453c5ad10/rio_cogeo/cogeo.py#L288-L290

vincentsarago commented 3 years ago

yep, I was going to push this fix! I'll cc you in the PR and wait for your 👍