Skip to content

Split SCons.Util even more, discourage implicit imports#4830

Closed
Repiteo wants to merge 3 commits intoSCons:masterfrom
Repiteo:util-no-implicit
Closed

Split SCons.Util even more, discourage implicit imports#4830
Repiteo wants to merge 3 commits intoSCons:masterfrom
Repiteo:util-no-implicit

Conversation

@Repiteo
Copy link
Copy Markdown
Contributor

@Repiteo Repiteo commented Feb 17, 2026

Makes SCons.Util more package-y with a couple of changes:

  • Disable type hints for SCons.Util's implicit imports. This keeps the functionality working as-is, but now intellisense and IDEs will discourage not using the correct package explicitly.
  • Split tests across the different packages, instead of having everything consolidated in UtilTests.py.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

Comment on lines -455 to +461
StringTypes=StringTypes,
SequenceTypes=SequenceTypes,
StringTypes=StringTypes, # type: ignore
SequenceTypes=SequenceTypes, # type: ignore
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Trying to use SCons.Util.sctypes.* here causes circular imports. Could maybe be circumvented with local import syntax, but it's simpler to just use the legacy values and silence the warnings.

@mwichmann
Copy link
Copy Markdown
Collaborator

I don't have a problem with this in concept. Away from big screens for a while, so is kind of hard to review. The new Tests.py files are all moved content, right? (UtilsTests has such a big diff it's collapsing it and not showing, means that'll be hard to tell visually).

@Repiteo
Copy link
Copy Markdown
Contributor Author

Repiteo commented Feb 18, 2026

It's all moved content, correct

except KeyError:
return None
if is_String(path):
if SCons.Util.sctypes.is_String(path):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are you making this change? So intellisence will work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For intellisense, yes. However, that was a bit silly when I could've just suppressed the warnings, so I did that instead

@Repiteo
Copy link
Copy Markdown
Contributor Author

Repiteo commented Mar 20, 2026

I'm no longer 100% convinced that this is the best solution, so I'll be marking this as a draft for now. Will move the test changes to another PR

@Repiteo Repiteo marked this pull request as draft March 20, 2026 16:24
is_list = True
paths = orig
if not is_List(orig) and not is_Tuple(orig):
if not SCons.Util.sctypes.is_List(orig) and not SCons.Util.sctypes.is_Tuple(orig):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you also remove SCons.Util.'s from this file?

@Repiteo
Copy link
Copy Markdown
Contributor Author

Repiteo commented Apr 12, 2026

In the interest of debloating PR count, I'll just close this outright. If I want to revisit the idea, it'll be in the form of a new PR

@Repiteo Repiteo closed this Apr 12, 2026
@github-project-automation github-project-automation bot moved this from In review to Done in Next Release Apr 12, 2026
@Repiteo Repiteo deleted the util-no-implicit branch April 12, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants