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

BLD: Add pyproject.toml #241

Merged
merged 17 commits into from
Oct 7, 2024
Merged

BLD: Add pyproject.toml #241

merged 17 commits into from
Oct 7, 2024

Conversation

DWesl
Copy link
Contributor

@DWesl DWesl commented May 24, 2024

This might close #237 by specifying NumPy as a build dependency before starting the setup script.

I moved as much configuration to pyproject.toml as I could as an initial step to converting the build to a system available on Python 3.12.

DWesl added 4 commits May 23, 2024 16:10
I think I got every key from setup.py.
The main difference is if the READTHEDOCS environment variable is set: I don't know how to change the dependencies based on that.

On the other hand, given 3.8 is close to or past EOL, I suspect 3.3 is relatively uncommon, and the RTD install, without the unneeded bits, could be accomplished with pip install --no-deps.
…tes.

numpy.distutils recommends setuptools<60, but that breaks pyproject.toml
configuration.  I like this way better.  Still needs setup.py for the
fortran module and for the Cheyenne configuration.
setuptools<60 just gets confused, and has no idea what the project name is.
I just inverted that requirement: not sure if setuptools needs to be more recent still.
@VascoSch92
Copy link

You could also deplace the requirements.txt inside the pyproject ;)

@DWesl
Copy link
Contributor Author

DWesl commented May 26, 2024

As in, delete line 55 of pyproject.toml and move the contents of requirements.txt into `[project].dependencies?

wrf-python/setup.py

Lines 61 to 73 in cec1f70

on_rtd = os.environ.get("READTHEDOCS", None) == "True"
# on_rtd=True
if on_rtd:
if sys.version_info < (3, 3):
requirements = ["mock"] # for python2 and python < 3.3
else:
requirements = [] # for >= python3.3
ext_modules = []
else:
# Place install_requires into the text file "requirements.txt"
with open("requirements.txt") as f2:
requirements = f2.read().strip().splitlines()

I meant to ask earlier how important installing only mock on RTD was.
The specification doesn't seem to allow conditioning requirements on environment variables, and I'm not sure TOML does either, unless there's a way for wrf-python[rtd] to have fewer dependencies than wrf-python.

@VascoSch92
Copy link

VascoSch92 commented May 26, 2024

As in, delete line 55 of pyproject.toml and move the contents of requirements.txt into `[project].dependencies?

Yes, probably you should also change pip install -r requirements.txt in pip install -r . everywhere

@DWesl
Copy link
Contributor Author

DWesl commented May 27, 2024

Yes, probably you should also change pip install -r requirements.txt in pip install -r . everywhere

It seems someone already did that: setup.py and MANIFEST.in were the only places to mention requirements.txt

DWesl added 2 commits May 28, 2024 10:20
The old setup script replaced the dependencies listed in requirements.txt with mock if python was old enough and nothing if python was recent.

The package no longer supports python versions that old (<3.3), so installing without dependencies is likely the correct way to handle the RTD configuration.
Forgot to remove this two or three commits back.
@kafitzgerald
Copy link
Collaborator

Thanks for the PR!

I was hoping to get this updated and merged yesterday, but need to need to look into why the Ubuntu CI checks are failing.
I'll try to take a deeper look later this week or next unless someone beats me to it.

NumPy suggested it, it might help with the distutils failure.
Let's see if that change is the one that broke things.
If it's just constants used somewhere, f2py drops the module.
Let's see if this gets Linux compiles working.
Copy link
Collaborator

@kafitzgerald kafitzgerald left a comment

Choose a reason for hiding this comment

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

Looks like the lingering failures were just a fluke w/ CI.

This all looks good to me.

@erogluorhan
Copy link
Collaborator

Great to see the issues of this pR have been fixed. Looks good to me

@erogluorhan erogluorhan merged commit fc2bc17 into NCAR:develop Oct 7, 2024
10 checks passed
@DWesl DWesl deleted the patch-2 branch October 8, 2024 15:54
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.

error when install wrf-python
4 participants