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

NameError: name 'tempita' is not defined when building extensions with old cython #28836

Closed
toobaz opened this issue Oct 8, 2019 · 10 comments · Fixed by #30498
Closed

NameError: name 'tempita' is not defined when building extensions with old cython #28836

toobaz opened this issue Oct 8, 2019 · 10 comments · Fixed by #30498
Labels
Build Library building on various platforms
Milestone

Comments

@toobaz
Copy link
Member

toobaz commented Oct 8, 2019

Code Sample, a copy-pastable example if possible

pietro@debiousci:~/nobackup/repo/pandas_temp$ python3 setup.py build_ext --inplace 
Traceback (most recent call last):
  File "setup.py", line 813, in <module>
    ext_modules=maybe_cythonize(extensions, compiler_directives=directives),
  File "setup.py", line 541, in maybe_cythonize
    build_ext.render_templates(_pxifiles)
  File "setup.py", line 127, in render_templates
    pyxcontent = tempita.sub(tmpl)
NameError: name 'tempita' is not defined

Problem description

If cython is too old/missing, the nice error reporting of the setup.py do not work.

Until few minutes ago I had never heard about tempita, but my understanding is that it is required (used in build_ext.render_templates) while cython is not:

cython = False

Hence, whether the check for tempita is made should not depend on whether cython is found: instead, this is what currently happens, given that tempita is checked in the else clause of the try...except check of cython:

try:

Notice that this means the above error pops out despite tempita being installed in my system.

Expected Output

None

Output of pd.show_versions()

Commit: bee17d5

@toobaz toobaz added the Build Library building on various platforms label Oct 8, 2019
@toobaz
Copy link
Member Author

toobaz commented Oct 8, 2019

(By the way: my command above will fail anyway because of the missing cython, but my understanding is that, because of this bug, setup.py will fail even when cython files are provided - and hence cython is not needed)

@jreback
Copy link
Contributor

jreback commented Oct 8, 2019

we bumped cython recently; conda update cython should be enough
this is a dev only dep and only if not creating a new from a new env

if you don’t have cython it actually would work

@toobaz
Copy link
Member Author

toobaz commented Oct 8, 2019

Yep, sorry for not mentioning I had already solved my local problem.

@jorisvandenbossche
Copy link
Member

I suppose this is caused by #28374 when we now require cython for sdists. Before, we only called cythonize when if cython, but now we assume cython is installed in maybe_cythonize. We can add a check there if cython/tempita is installed and otherwise raise a better error message?

@toobaz
Copy link
Member Author

toobaz commented Oct 8, 2019

I think (but I'm quite ignorant of how the setup.py works) that just moving the following try..except

try:

... out of the else should fix this...

@WillAyd
Copy link
Member

WillAyd commented Oct 8, 2019

I think that block could be simplified - the pip install Tempita command I don't believe is valid any longer.

I generally think anything to simplify setup.py would be great. I would be hesitant to add more logic

@jorisvandenbossche
Copy link
Member

@toobaz I don't think that should be moved out of the else clause, as in theory you don't need tempita to run setup.py (eg sdist does not need it).

But I think @WillAyd is correct that the import tempita / pip install tempita is no longer needed, it might stem from a time that we still supported cython versions that did not include tempita

@toobaz
Copy link
Member Author

toobaz commented Oct 9, 2019

@toobaz I don't think that should be moved out of the else clause, as in theory you don't need tempita to run setup.py (eg sdist does not need it).

Right, it would need also a variable where we remember whether tempita is present, like for cython.

But I think @WillAyd is correct that the import tempita / pip install tempita is no longer needed, it might stem from a time that we still supported cython versions that did not include tempita

Again, I'm not an expert, but my understanding is that if you have tempita but not cython, the setup.py still tried to allow you to take a pandas provided with cython files and compile them to C. But then, this is a very marginal use case, so maybe not worth the effort.

@jorisvandenbossche
Copy link
Member

if you have tempita but not cython, the setup.py still tried to allow you to take a pandas provided with cython files and compile them to C

tempita itself is not enough to compile cython file to C (tempita only generates cython files from the templates, not C files), so that use case doesn't sound possible to me.

So I think it should be fine to just rely on tempita provided by cython?

@toobaz
Copy link
Member Author

toobaz commented Oct 9, 2019

tempita itself is not enough to compile cython file to C (tempita only generates cython files from the templates, not C files), so that use case doesn't sound possible to me.

OK sorry, now I understand, and sure, checking ``tempita'' is useless.

jbrockmendel added a commit to jbrockmendel/pandas that referenced this issue Dec 27, 2019
@jreback jreback added this to the 1.0 milestone Dec 27, 2019
keechongtan added a commit to keechongtan/pandas that referenced this issue Dec 29, 2019
…ndexing-1row-df

* upstream/master: (333 commits)
  CI: troubleshoot Web_and_Docs failing (pandas-dev#30534)
  WARN: Ignore NumbaPerformanceWarning in test suite (pandas-dev#30525)
  DEPR: camelCase in offsets, get_offset (pandas-dev#30340)
  PERF: implement scalar ops blockwise (pandas-dev#29853)
  DEPR: Remove Series.compress (pandas-dev#30514)
  ENH: Add numba engine for rolling apply (pandas-dev#30151)
  [ENH] Add to_markdown method (pandas-dev#30350)
  DEPR: Deprecate pandas.np module (pandas-dev#30386)
  ENH: Add ignore_index for df.drop_duplicates (pandas-dev#30405)
  BUG: The setting xrot=0 in DataFrame.hist() doesn't work with by and subplots pandas-dev#30288 (pandas-dev#30491)
  CI: Fix GBQ Tests (pandas-dev#30478)
  Bug groupby quantile listlike q and int columns (pandas-dev#30485)
  ENH: Add ignore_index for df.sort_values and series.sort_values (pandas-dev#30402)
  TYP: Typing hints in pandas/io/formats/{css,csvs}.py (pandas-dev#30398)
  BUG: raise on non-hashable Index name, closes pandas-dev#29069 (pandas-dev#30335)
  Replace "foo!r" to "repr(foo)" syntax pandas-dev#29886 (pandas-dev#30502)
  BUG: preserve EA dtype in transpose (pandas-dev#30091)
  BLD: add check to prevent tempita name error, clsoes pandas-dev#28836 (pandas-dev#30498)
  REF/TST: method-specific files for test_append (pandas-dev#30503)
  marked unused parameters (pandas-dev#30504)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants