Use relative imports in matter.idl module #38196
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
documentation
|
PR #38196: Size comparison from bd2b4f5 to 6b9b384 Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #38196: Size comparison from bd2b4f5 to e4bad1b Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
andy31415
reviewed
Apr 2, 2025
andy31415
reviewed
Apr 2, 2025
| from .generators.generator import FileSystemGeneratorStorage, GeneratorStorage | ||
| from .matter_idl_parser import CreateParser | ||
|
|
||
| __all__ = [ |
There was a problem hiding this comment.
Why did we flatten the exported parts? In particular:
backwards_compatibilityseems cleaner thancompatibility(compatibility with what)- IDL generally is the
matter files. Pulling XMLs is a bit unclear as those are different types and should probably be separate namespace (especially for ParseSource ... what source) typesseems a bit generic and likely to name clash
The others seem somewhat reasonable, however overally "why?"
There was a problem hiding this comment.
The others seem somewhat reasonable, however overally "why?"
I've created this top level imports in order to simplify mater.idl imports in case of using this package directly from the source (without installing). Then the import can look like this:
try:
from matter import idl
except ImportError:
sys.path.append(str(Path(__file__).resolve().parent / "py_matter_idl" / "matter"))
import idlAnyway, if it seems off for the idl package to have top level imports I will remove it. The try/except will be longer, though.
There was a problem hiding this comment.
I've simply pulled all submodules into the matter.idl without any aliasing. So, in the scripts we can do simple from matter import idl and hopefully every class/function usage will be clear now.
|
PR #38196: Size comparison from 86e1daf to c89b15b Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
During transition to
matternamespace it will be necessary to install all modules in a single location, otherwise Python will not be able to import modules properly. As a workaround for using package without installation instead of importing frommatternamespace we can import the module directly, i.e.from matter.idl import ...->from idl import .... To do that in build scripts we need to adjust import paths accordingly. Also, we have to use relative imports inmatter.idlto allowfrom idl import ....Changes
matter.idlmodulescriptsdirectory outside of thematter.idlmodule - this allows to run scripts if the matter-idl package is not installed after transition to relative importsmatter.idl.generatorsmodule to break circular dependencyWarning
The names and location of the
matter.idlscripts were changed. Now all command line tools are available atscripts/py_matter_idl/scripts/*.py.Testing
CI will verify