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

Mau uses __dunder__ names that are reserved by Python #11

Open
whitequark opened this issue Apr 11, 2024 · 4 comments
Open

Mau uses __dunder__ names that are reserved by Python #11

whitequark opened this issue Apr 11, 2024 · 4 comments

Comments

@whitequark
Copy link
Member

I found code in mau like this:

    def __option_parser_result__(self, proto: OptionParser[T]) -> T:

Unfortunately this is actually undefined behavior; Python reserves all __x__ names for itself. You might want to use _option_parser_result_ instead, though this would be bizarre and unconventional. There is also no reason to use __names, since they provide absolutely no extra protection--you can still access _ClassName__names from the outside like with any other attribute. The __name syntax exists for collision avoidance, not safety.

The conventional way would be to use a single prefixed underscore for all of the private functions and methods, since this provides the maximum possible safety (none, other than being labelled "do not touch") and uses conventional syntax.

@jix
Copy link
Member

jix commented Apr 11, 2024

The use of __ prefixes for collision avoidance (outside of the __dunder__ methods) is intentional and should be limited to places where the intended use of the API involves user code implementing subclasses, i.e. where there is an actual collision risk.

I'll think about what to do about the __dunder__ methods.

@whitequark
Copy link
Member Author

The use of __ prefixes for collision avoidance (outside of the __dunder__ methods) is intentional and should be limited to places where the intended use of the API involves user code implementing subclasses, i.e. where there is an actual collision risk.

Ah, I see. In my (admittedly incomplete) review I couldn't see a single file without using __ prefixes so I jumped to a conclusion.

By the way (I am reviewing mau with my packager hat on here)--is the plan to put it on PyPI and depend on it in Yosys itself? Or is the plan to vendor it? I can see both plans to have some undesirable consequences and I'd rather know sooner than later.

@jix
Copy link
Member

jix commented Apr 11, 2024

There are currently no plans to add this as dependency for yosys (or to any of the python programs in the main yosys repo like yosys-smtbmc). The plan is to eventually migrate all of our frontend tools (SBY, EQY, etc.) to use this, some newer and/or experimental frontend tools already use mau. I'd be in favor of eventually releasing mau as well as these frontend tools themselves also on PyPI, but still have to discuss how that would fit into our release process (cc @mmicko).

@whitequark
Copy link
Member Author

Thanks, this actually clears up my concerns (which were mainly about Yosys itself, as that is a hybrid C++/Python application that is a considerable pain to package as such), so feel free to ignore this line of questioning.

Pure Python applications are much easier to package for Wasm: either it uses subprocesses, in which case I just don't, or it doesn't, in which case pyodide can usually install and run it as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants