Skip to content

Remove star import from _mkl_service #178

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

Open
ndgrigorian wants to merge 7 commits into
base: master
Choose a base branch
from
+103 −6
Open

Remove star import from _mkl_service #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
34e1f8c
remove star import in __init__
ndgrigorian Apr 10, 2026
b8584cc
add black and isort settings to pyproject.toml
ndgrigorian Apr 11, 2026
5045ed9
fix setup.py for linter
ndgrigorian Apr 11, 2026
ff26c3b
lazily import sys in RTLD_for_MKL
ndgrigorian Apr 11, 2026
b0752ec
add setup.py linting commit to git-blame-ignore
ndgrigorian Apr 11, 2026
d68825c
add gh-178 to changelog
ndgrigorian Apr 12, 2026
581e54f
Merge branch 'master' into remove-star-import
ndgrigorian Apr 12, 2026
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
3 changes: 3 additions & 0 deletions .git-blame-ignore-revs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@

# Move to clang-format-22
6056b26ff82dfd156708fd7d3680470d9ba3c9ce

# Clean up in setup.py
5045ed9c6b39fa1747cdae2719477e5a596f0d3b
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Dropped support for Python 3.9 [gh-118](https://github.com/IntelPython/mkl-service/pull/118)
* Dropped support for `"ssse3"`, `"sse4_1"`, `"avx"`, `"avx512_mic"`, `"avx512_mic,strict"`, and `"avx512_mic_e1"` cbwr branches [gh-173](https://github.com/IntelPython/mkl-service/pull/173)

### Fixed
* Removed use of star import in `mkl-service` initialization, which removes imported module pollution from the namespace [gh-178](https://github.com/IntelPython/mkl-service/pull/178)

## [2.6.1] (11/25/2025)

### Fixed
Expand Down
84 changes: 80 additions & 4 deletions mkl/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

import sys

from . import _init_helper


Expand All @@ -34,6 +32,7 @@ def __init__(self):

def __enter__(self):
import ctypes
import sys
Copy link
Copy Markdown
Collaborator

@antonwolfy antonwolfy Apr 13, 2026

Choose a reason for hiding this comment

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

Small question/concern: why lazy sys import is preferable?

  • Lazy imports inside methods are a Python anti-pattern for standard library modules
  • No real benefit - sys is lightweight and commonly expected in module namespaces
  • Adds unnecessary overhead (minimal but pointless)
  • Makes code harder to understand

The explicit __all__ list already defines the public API, so having sys at module level seems fine.


try:
self.saved_rtld = sys.getdlopenflags()
Expand All @@ -46,6 +45,8 @@ def __enter__(self):
del ctypes

def __exit__(self, *args):
import sys

if self.saved_rtld:
sys.setdlopenflags(self.saved_rtld)
self.saved_rtld = None
Expand All @@ -55,9 +56,84 @@ def __exit__(self, *args):
from . import _mklinit

del RTLD_for_MKL
del sys

from ._py_mkl_service import *
from ._py_mkl_service import (
Copy link
Copy Markdown
Collaborator

@antonwolfy antonwolfy Apr 13, 2026

Choose a reason for hiding this comment

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

Can we remove F403 from flake8 ignore config?

cbwr_get,
cbwr_get_auto_branch,
cbwr_set,
disable_fast_mm,
domain_get_max_threads,
domain_set_num_threads,
dsecnd,
enable_instructions,
free_buffers,
get_clocks_frequency,
get_cpu_clocks,
get_cpu_frequency,
get_dynamic,
get_env_mode,
get_max_cpu_frequency,
get_max_threads,
get_num_stripes,
get_version,
get_version_string,
mem_stat,
peak_mem_usage,
second,
set_dynamic,
set_env_mode,
set_memory_limit,
set_mpi,
set_num_stripes,
set_num_threads,
set_num_threads_local,
thread_free_buffers,
verbose,
vml_clear_err_status,
vml_get_err_status,
vml_get_mode,
vml_set_err_status,
vml_set_mode,
)
from ._version import __version__

__all__ = [
"get_version",
"get_version_string",
"set_num_threads",
"domain_set_num_threads",
"set_num_threads_local",
"set_dynamic",
"get_max_threads",
"domain_get_max_threads",
"get_dynamic",
"set_num_stripes",
"get_num_stripes",
"second",
"dsecnd",
"get_cpu_clocks",
"get_cpu_frequency",
"get_max_cpu_frequency",
"get_clocks_frequency",
"free_buffers",
"thread_free_buffers",
"disable_fast_mm",
"mem_stat",
"peak_mem_usage",
"set_memory_limit",
"cbwr_set",
"cbwr_get",
"cbwr_get_auto_branch",
"enable_instructions",
"set_env_mode",
"get_env_mode",
"verbose",
"set_mpi",
"vml_set_mode",
"vml_get_mode",
"vml_set_err_status",
"vml_get_err_status",
"vml_clear_err_status",
]

del _init_helper
11 changes: 11 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,17 @@ test = ["pytest"]
Download = "http://github.com/IntelPython/mkl-service"
Homepage = "http://github.com/IntelPython/mkl-service"

[tool.black]
line-length = 80

[tool.isort]
ensure_newline_before_comments = true
force_grid_wrap = 0
include_trailing_comma = true
line_length = 80
multi_line_output = 3
use_parentheses = true
Comment on lines +79 to +84
Copy link
Copy Markdown
Collaborator

@antonwolfy antonwolfy Apr 13, 2026

Choose a reason for hiding this comment

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

Would it be enough to use:

Suggested change
ensure_newline_before_comments = true
force_grid_wrap = 0
include_trailing_comma = true
line_length = 80
multi_line_output = 3
use_parentheses = true
line_length = 80
profile = "black"

Seems other settings can be omitted with profile = "black"


[tool.setuptools]
include-package-data = true
packages = ["mkl"]
Expand Down
8 changes: 6 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ def extensions():
if mkl_root:
mkl_info = {
"include_dirs": [join(mkl_root, "include")],
"library_dirs": [join(mkl_root, "lib"), join(mkl_root, "lib", "intel64")],
"library_dirs": [
join(mkl_root, "lib"),
join(mkl_root, "lib", "intel64"),
],
"libraries": ["mkl_rt"],
}
else:
Expand All @@ -62,7 +65,8 @@ def extensions():
"mkl._mklinit",
sources=[join("mkl", "_mklinitmodule.c")],
include_dirs=mkl_include_dirs,
libraries=mkl_libraries + (["pthread"] if os.name == "posix" else []),
libraries=mkl_libraries
+ (["pthread"] if os.name == "posix" else []),
library_dirs=mkl_library_dirs,
runtime_library_dirs=mkl_rpaths,
extra_compile_args=[
Expand Down
Loading