Skip to content

DEV: Update .gitpod.yml to build inplace #52719

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 18, 2023
Merged

DEV: Update .gitpod.yml to build inplace #52719

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitpod.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ tasks:
conda activate pandas-dev
git pull --unshallow # need to force this else the prebuild fails
git fetch --tags
python setup.py build_ext -j 4
python setup.py build_ext --inplace -j 4
Copy link
Member

Choose a reason for hiding this comment

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

isn't inplace already in pyproject.toml? just for my understanding, why is this necessary?

Copy link
Member

Choose a reason for hiding this comment

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

in https://github.com/pandas-dev/pandas/pull/49917/files,

[build_ext]
inplace = True

in setup.cfg was changed to

# TODO: Remove after setuptools support is dropped.
[tool.setuptools]
include-package-data = true

in pyproject.toml

do they do the same thing?

the docs don't have --inplace

pandas/doc/source/development/contributing_environment.rst

Line 215 in bfa793c

python setup.py build_ext -j 4

so does this need updating there?

Copy link
Member

Choose a reason for hiding this comment

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

@lithomas1 sorry for the ping, just trying to understand the above

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the ping. This does indeed seem missing, but the old syntax seems to be distutils specific since I can't find anything on it in the setuptools docs.
(see https://docs.python.org/3/distutils/configfile.html).

Maybe we can update where we are overriding build_ext in setup.py to also pass through inplace=True as the default?
(here

pandas/setup.py

Lines 76 to 104 in 11d75d8

class build_ext(_build_ext):
@classmethod
def render_templates(cls, pxifiles):
for pxifile in pxifiles:
# build pxifiles first, template extension must be .pxi.in
assert pxifile.endswith(".pxi.in")
outfile = pxifile[:-3]
if (
os.path.exists(outfile)
and os.stat(pxifile).st_mtime < os.stat(outfile).st_mtime
):
# if .pxi.in is not updated, no need to output .pxi
continue
with open(pxifile) as f:
tmpl = f.read()
pyxcontent = Tempita.sub(tmpl)
with open(outfile, "w") as f:
f.write(pyxcontent)
def build_extensions(self):
# if building from c files, don't need to
# generate template output
if _CYTHON_INSTALLED:
self.render_templates(_pxifiles)
super().build_extensions()
?)

I'm not really inclined to change this, so a doc update should suffice (since this time, the meson build work should actually be ready, just waiting on a meson-python release).

Copy link
Member

Choose a reason for hiding this comment

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

as in, a doc update to add --inplace to the build commands? sure, sounds good

(since this time, the meson build work should actually be ready, just waiting on a meson-python release).

ooh, exciting!

echo "🛠 Completed rebuilding Pandas!! 🛠 "
echo "✨ Pre-build complete! You can close this terminal ✨ "

Expand Down