search

avaframe / AvaFrame

https://avaframe.org
European Union Public License 1.2
16 stars 2 forks source link

Make syntax compatible with NumPy 2.0 #1007

Closed ninsbl closed 2 months ago

ninsbl commented 3 months ago

There are some lines of NumPy code that are not compatible with NumPy 2.0. This PR addresses those.

You can check it with 

python3 -m pip install ruff
ruff check --preview --select NPY

(ruff is a nice and comprehensive linter that I can recomment BTW...)

Since there is no avaframe package available with NumPy 2.0 support yet, I tried to pip-install avaframe from source, but that failed, unfortunately. Reason is that c-files were not created with

python3 -m pip install git+https://github.com/avaframe/AvaFrame

. I tried to address that in setup.py, and with this PR it should work, see:

python3 -m pip install git+https://github.com/ninsbl/AvaFrame@np2

...

In order to get a PEP404 compliant version I had to import the tags to my fork though:

# Clone my fork
git clone git@github.com:ninsbl/AvaFrame
# Enter my working copy
cd AvaFrame
# Add AvaFrame as upstream reference
git remote add upstream https://github.com/avaframe/AvaFrame
# Fetch tags
git fetch --tags upstream
# Push tags to my fork
git push --tags

Maybe worth adding that in the developer documentation somewhere...

pep8speaks commented 3 months ago

Hello @ninsbl! Thanks for updating this PR.

Line 315:5: E303 too many blank lines (2) Line 169:14: E127 continuation line over-indented for visual indent Line 167:14: E127 continuation line over-indented for visual indent Line 165:14: E127 continuation line over-indented for visual indent Line 163:14: E127 continuation line over-indented for visual indent Line 161:14: E127 continuation line over-indented for visual indent Line 159:14: E127 continuation line over-indented for visual indent Line 148:14: E127 continuation line over-indented for visual indent Line 144:14: E127 continuation line over-indented for visual indent Line 142:14: E127 continuation line over-indented for visual indent

Line 203:21: W292 no newline at end of file

Comment last updated at 2024-08-09 10:01:50 UTC
ninsbl commented 3 months ago

Test failures do not seem to be related to the changes in this PR.

Is the plan to implement code style incrementaly with every changed file? If so, do you have any hints on how to fix code style issues with codeclimate.

fso42 commented 2 months ago

Hi @ninsbl, thanks for your PR! Sorry for the late reply, I was on vacation. I will look into this this week.

fso42 commented 2 months ago

@ninsbl: Jup, the test failures are unrelated to your PR. The first ones are fixed (I rebased on the current master), but new ones cropped up. I'm looking into this atm...

@ the issues with code climate: we use this only loosely to get an idea of what needs to be looked at; it is a bit too sensitive for our liking. So please don't put too much emphasis on it :-).

@ the code formatting: We are waiting for a good point in time to format the whole repo with black at once (because then it would be possible to ignore that commit), but at the moment, too much development is going on.

As to the pyproject.toml vs setup.py: I'd love to move away from setup.py, but at the moment, due to Cython, this is not yet possible. At least I haven't found a solution, maybe you got an idea? (https://github.com/pypa/setuptools/discussions/4154)

fso42 commented 2 months ago

Ah, the cc reporter problem is this here : https://github.com/IBM/scc-go-sdk/issues/71#issuecomment-1010639476 , so we can ignore this... (Our codeclimate secret is not exposed on forked branches...)

fso42 commented 2 months ago

Standardtest ok, apart from known.

fso42 commented 2 months ago

@ninsbl : Thanks again for your PR!

ninsbl commented 2 months ago

Thanks for merging!

@ the code formatting: We are waiting for a good point in time to format the whole repo with black at once (because then it would be possible to ignore that commit), but at the moment, too much development is going on.

This is how GRASS GIS introduced black (and flake8): https://github.com/OSGeo/grass/issues/543

Currently, there is also an effort to make the code base ruff compliant (https://github.com/OSGeo/grass/issues/4028, https://github.com/OSGeo/grass/issues/3921), which resulted in a lot of code style improvements (and few regressions). Formating all in one commit can make it harder to debug evtl regressions...

Just in case you are interested in some inspiration for that endevour...