[Bug 2003287] Review Request: python-llvmlite - A lightweight LLVM python binding for writing JIT compilers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=2003287



--- Comment #3 from Ankur Sinha (FranciscoD) <sanjay.ankur@xxxxxxxxx> ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
> It's great that you're working on this. Having Numba in Fedora would be
> wonderful: for
> me it's one of the important remaining pieces for scientific Python work
> with a pure
> Fedora installation.

+1: a few of the packages on our Neuro-SIG queue also need numba, so it'll be
good to have it packaged up

> 
> > URL:            http://numba.pydata.org/
> https://

Corrected

> 
> The %description is written for a person who already knows what llvmlite is
> and is justifying
> its existence. We need an explanation for people who have no idea. Maybe
> something like
> this (and I'm paraphrasing the existing text based on my understanding only):
> 
>   llvmlite is a wrapper around the llvm compiler created for Numba to
> interact with the internal
>   llvm internation representation of code. Only the IR builder, optimizer,
> and JIT compiler APIs
>   are covered.
> 
>   llvmlite consists of a small C wrapper around some parts of the LLVM C++
> API that are not
>   already exposed by the LLVM C API, a ctypes wrapper around the C API to
> allow access
>   from Python code, and a Python implementation of the subset of the LLVM IR
> builder that is
>   needed for Numba.
> 
> The "Key Benefits" part could be included, but I'm not sure if it's useful
> to the users
> of the package.

I've copied bits from here now:

https://llvmlite.readthedocs.io/en/latest/#philosophy

So it now says:

llvmlite provides a Python binding to LLVM for use in Numba.

Numba previously relied on llvmpy.  While llvmpy exposed large parts of the
LLVM C++ API for direct calls into the LLVM library, llvmlite takes an entirely
different approach. Llvmlite starts from the needs of a JIT compiler and splits
them into two decoupled tasks:

- Construction of a Module, function by function, Instruction by instruction.
- Compilation and optimization of the module into machine code.

The construction of an LLVM module does not call the LLVM C++ API. Rather, it
constructs the LLVM intermediate representation (IR) in pure Python. This is
the role of the IR layer.

The compilation of an LLVM module takes the IR in textual form and feeds it
into LLVM's parsing API. It then returns a thin wrapper around LLVM's C++
module object. This is the role of the binding layer.

Once parsed, the module's source code cannot be modified, which loses the
flexibility of the direct mapping of C++ APIs into Python that was provided by
llvmpy but saves a great deal of maintenance.



> 
> > %check
> 
> I think the tests should be enabled by default.
> This is especially important when we're not using the same llvm version as
> upstream.
> When enabled, they pass here without any issue.
> (Also, maybe add '-v' or equivalent so we get a better log of what was
> executed…)
> 
> > LD_LIBRARY_PATH="$LD_LIBRARY_PATH:%{buildroot}/%{python3_sitearch}/llvmlite/binding/" PYTHONPATH="$PYTHONPATH:%{buildroot}/%{python3_sitearch}:%{buildroot}%{python3_sitelib}" %{__python3} runtests.py

> 
> Maybe:
> LD_LIBRARY_PATH="%{buildroot}%{python3_sitearch}/llvmlite/binding/"
> PYTHONPATH="$PYTHONPATH:%{buildroot}%{python3_sitearch}:
> %{buildroot}%{python3_sitelib}" %{python3} runtests.py
> (less "/", we expect LD_LIBRARY_PATH to be unset, %python3 is now preferred)
> 

Enabled tests, updated the command, and increased the verbosity too.

> '%license LICENSE' is missing.

I've learned that it isnt needed since it gets included by the
pyproject_save_files macro:

rpm -ql --licensefiles -p RPMS/python3-llvmlite-0.37.0-1.fc36.x86_64.rpm 
/usr/lib64/python3.10/site-packages/llvmlite-0.37.0.dist-info/LICENSE

> 
> + package name is OK
> + latest version
> + license is acceptable for Fedora (BSD)
> + license is specified correctly
> + builds and installs OK
> + fedora-review and rpmlint find no issues (except bogus spelling
> suggestions and %autorelease misunderstandings)
> - %check is disabled
> + BR/R/BR look correct
> - license is missing from %files
> 
> Package is APPROVED, but please fix the minor issues pointed out above when
> importing.


Thanks, requested SCM now.

Updated spec/srpm:

Spec URL:
https://ankursinha.fedorapeople.org/python-llvmlite/python-llvmlite.spec
SRPM URL:
https://ankursinha.fedorapeople.org/python-llvmlite/python-llvmlite-0.37.0-1.fc34.src.rpm


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux