search

KipCrossing / geotiff

A noGDAL tool for reading and writing geotiff files
GNU Lesser General Public License v2.1
216 stars 23 forks source link

Update tifffile>=2023.4.12 in requirements.txt #66

Closed doyled-it closed 1 year ago

doyled-it commented 1 year ago

Closes #62

I ran the code in #62 with this tifffile version installed and I received the same output.

Zeitsperre commented 1 year ago

The bad version appears to have been tifffile@v2022.4.26. I'll need to go through the changelogs here and there to figure out what we should be pinning.

I'm tempted to try testing against several versions of tiffile to see exactly which versions should be avoided.

Feel free to ignore the Python3.9 failure (will be addressed in #65).

doyled-it commented 1 year ago

The bad version appears to have been tifffile@v2022.4.26. I'll need to go through the changelogs here and there to figure out what we should be pinning.

I'm tempted to try testing against several versions of tiffile to see exactly which versions should be avoided.

Feel free to ignore the Python3.9 failure (will be addressed in #65).

I was just told by a colleague @mdotter-mitre that even version 2023.3.21 of tifffile produces the same error, so yeah, it may require going through more versions.

mdotter-mitre commented 1 year ago

It looks like from this issue, any version below and including tifffile<=2023.3.21 will result on the NODATA error, not isolated in tifffile==2022.4.26. Recommend setting tifffile>=2023.4.12.

Zeitsperre commented 1 year ago

@doyled-it @mdotter-mitre

Thanks for pointing this out.

I'm reluctant to break backwards compatibility (recent versions of tifffile drop both Python3.7 and 3.8), so maybe we can make use of PEP 508's environment markers, i.e.

tifffile>=2021.7.4,<the.version.where.it.all.went.wrong;python_version>=3.7,<3.9
tifffile>=2023.4.12;python_version>=3.9

Do we know for certain that the regression was introduced in tifffile==2022.4.26, or did it happen earlier?

KipCrossing commented 1 year ago

Thanks for looking into this. I'll check it out when I'm up (Aus time).

@Zeitsperre I'll give you ownership of this repo and the PyPI so you can update at will.

Also, looks like mypy is failing in version in 3.9.

Zeitsperre commented 1 year ago

@KipCrossing Thanks so much! My PyPI username is the same as on here. I'll do my best to not make any egregious packaging errors!

AFAIK, the mypy error is addressed here: https://github.com/KipCrossing/geotiff/pull/65/commits/fa6598ca84be872f2e89e24bd6526aa18db8a70b

KipCrossing commented 1 year ago

@doyled-it do you mind pulling the latest main into this branch?

KipCrossing commented 1 year ago

@Zeitsperre I've added you to PyPI. Is there any additional permissions that you need here on GitHub so, if I'm AFK for a while you can do everything?

In general, it would be good to get more contributes on this project. The UI is pretty ugly and could use an overhaul. I'm not really in the GIS space these days so I don't have any strong opinions, tho.

(sorry for dumping this in the PR haha)

doyled-it commented 1 year ago

@doyled-it do you mind pulling the latest main into this branch?

@KipCrossing I'm kinda new to GitHub PRs and forking as we mostly work with GitLab MRs, so I hope I did that correct here.

KipCrossing commented 1 year ago

@KipCrossing I'm kinda new to GitHub PRs and forking as we mostly work with GitLab MRs, so I hope I did that correct here.

Perfect! I'll merge to main and fix the typing problem before deploying to PyPI