search

tbenthompson / okada_wrapper

MATLAB and Python wrappers of the Okada Green's functions.
MIT License
67 stars 33 forks source link

Use meson to build extension #11

Closed tobiscode closed 5 months ago

tobiscode commented 5 months ago

Hi, I was able to track the error to some unintuitive (or buggy) behavior in how meson and/or f2py use the .pyf file. Specifically, it seems that the part where it specifies the output of the DC3D(0) functions is at least partially ignored. By adding the intent as a comment into the source .f file, the expected behavior is restored. (I'm not sure whether the .pyf file is still needed, but I kept it in for now.) I took the liberty of addressing some linter messages and removing the LaTeX dependency for test_okada.py to make it easier for people to test everything, please feel free to ignore these parts of the PR if desired. I hope this fixes https://github.com/tbenthompson/okada_wrapper/issues/8 ; I've tested it with Python 3.12 myself and the installation and and test_okada.py work now. Cheers

tobiscode commented 5 months ago

Oh, I didn't change the version number, that's maybe something to consider to make it clearer for pip installations to see it was updated and now has different requirements.

brendanjmeade commented 5 months ago

Wow! This is fantastic. Is it the python version that needs to be updated in environment.yml? Something else?

tbenthompson commented 5 months ago

@tobiscode This is fantastic!! Thank you so much. I'm going to give this PR a look over as soon as I can but I'll probably just merge it. Then I'll release a new version. It might be this week but it might take until this weekend because I'm on a work trip and it's quite busy.

@brendanjmeade If I remember correctly, there are several places. environment.yml and setup.py come to mind.

tbenthompson commented 5 months ago

Ok, I got excited and looked through! All great stuff. Thanks again.

tobiscode commented 5 months ago

Awesome! Since setup.py isn't a thing anymore, the only place to update the package version is pyproject.toml (version = "18.12.08.3"). For the users, nothing should change since all changes are backwards-compatible / enforced by the use of the meson backend, so the part requires-python = ">=3.8" inside pyproject.toml can probably just stay. I don't see an environment.yml file but that is probably also fine.

brendanjmeade commented 5 months ago

Thanks Tobias! That makes a lot of sense.