BLD: fix bleeding-edge build script

[yut23]

PR Summary

Updates the bleeding-edge CI workflow script to run only pip and adds the numpy API macros to the clib defines.

Fixes #4896.

PR Checklist

• New features are documented, with docstrings and narrative docs
• Adds a test for any bugs fixed. Adds tests for new features.

[yut23]

I'm not sure if python setup.py build_ext is necessary anymore, since the build also works fine if I remove the setup.py calls entirely.

[neutrinoceros]

Thanks a lot for following up on this so promptly !

I'm not sure if python setup.py build_ext is necessary anymore, since the build also works fine if I remove the setup.py calls entirely.

You're right, the script is currently mixing up 2 ways to build yt, and pip install already does build + install so setup.py invokes are redundant (and apparently broken) !

There's also a pre-commit lint error (though I don't get why it's poping now).

[yut23]

Is python setup.py develop still a supported install method? The docs mention it here: https://yt-project.org/docs/dev/visualizing/unstructured_mesh_rendering.html. If it is, then we should probably do something like python-hyper/brotlicffi#92 instead, as develop only runs build_ext, not build_clib.

[yut23]

yt update also uses setup.py, see update_git() and rebuild_modules() in yt/funcs.py:

yt/yt/funcs.py

Lines 474 to 481 in 8d58839

	print("Here's a set of sample commands:")
	print("")
	print(f" $ cd {path}")
	print(" $ git stash")
	print(" $ git checkout main")
	print(" $ git pull")
	print(" $ git stash pop")
	print(f" $ {sys.executable} setup.py develop")

yt/yt/funcs.py

Lines 515 to 522 in 8d58839

	def rebuild_modules(path, f):
	f.write("Rebuilding modules\n\n")
	p = subprocess.Popen(
	[sys.executable, "setup.py", "build_ext", "-i"],
	cwd=path,
	stdout=subprocess.PIPE,
	stderr=subprocess.STDOUT,
	)

[neutrinoceros]

nice catch. I don't know that we exercise setup.py develop anywhere else and I don't know if it still works as intended. pip install unfortunately does a bit more than what rebuild_modules is supposed to do so I think we should stick to setup.py commands in this function and include the clibs build step your first proposed there.
FTR I'm not sure anyone still relies on the yt command line to manage a git repo (or that anyone should), so I would advocate for deprecating them, but that's out of scope for the present PR.

[neutrinoceros]

"Mergeable bot" appears to be stuck. As I don't control it, I'm going to try to close/reopen, hoping this wakes it up.

[matthewturk]

Ah, this seems to be related to where I was running into issues. I believe my current development workflow is broken, as I utilize build_ext -i constantly and have multiple checkouts I switch between. I find it annoying that it's not working, but unless I am able to identify a solution I won't belabor the point here and I'll just try to update my usage.

[neutrinoceros]

It still works if you run build_clib first.

[matthewturk]

Oh, awesome. Thank you :)

…

On Thu, May 16, 2024 at 12:24 PM Clément Robert ***@***.***> wrote: It still works if you run build_clib first. — Reply to this email directly, view it on GitHub <#4898 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAVXOY2QOGDFSPMIRGCOFTZCTTUPAVCNFSM6AAAAABHLOZGMWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJVHAYTMMJRGY> . You are receiving this because you commented.Message ID: ***@***.***>