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

Unnecessary underscore suffix for many arguments and attributes #234

Open
wbolster opened this Issue Nov 25, 2014 · 8 comments

Comments

Projects
None yet
3 participants
@wbolster

wbolster commented Nov 25, 2014

In Python, id is not a reserved word (it is the name of a built-in function though), so there is no reason to use id_ for function arguments and attribute names. It only clutters the API.

The same applies to type and object.

@wbolster wbolster changed the title from "id" is not a reserved word, so why use "id_" everywhere? to Unnecessary underscore suffix for many arguments and attributes Nov 25, 2014

@gtback

This comment has been minimized.

Contributor

gtback commented Nov 25, 2014

Thanks, @wbolster . This is something I'd like to consider the next time we do a major refactoring of the library (perhaps for the next major release of the language).

I was initially opposed to having attributes with the same name as Python builtins (particularly ones that actually get used in the same code, such as object and type). In addition, it has the potential to make syntax highlighters confused, which can in turn confuse programmers. Since then, I've seen more occasions where libraries use (e.g.) id as an attribute/kwarg, so "overriding" the builtin may not be as bad of an idea as I initially thought.

I'm curious to hear what other people think. Does anyone know of examples (popular Python libraries) where the "trailing _" strategy is vs. is not used?

Regardless, I don't think we'll want to change it until a major release. The backward compatibility issues would be enormous.

@gtback

This comment has been minimized.

Contributor

gtback commented Nov 25, 2014

My initial confusion was caused by mentally substituting "built-in" for "keyword" in PEP8.

single_trailing_underscore_ : used by convention to avoid conflicts with Python keyword, e.g.

I'll take the blame 100% for this.

@wbolster

This comment has been minimized.

wbolster commented Nov 25, 2014

Thanks. Trailing underscores are a necessary evil for things like class_ (prefer cls) and from_ (e.g. in an email api), but built-ins can be overridden. It allows nice APIs at the cost of a bit of careful coding inside the library,. e.g. alias builtin_type = type when using both a type=... arg and the type() built-in.

That said, do you have any other "big picture" ideas for a future version?

@wbolster

This comment has been minimized.

wbolster commented Nov 25, 2014

Re. example code: all Pythonic code has nice APIs of course! :) For example, it's very common to have a id field on SQLAlchemy models. An example for a type arg in the stdlib (among many others): https://docs.python.org/3/library/mimetypes.html#mimetypes.guess_extension

@gtback

This comment has been minimized.

Contributor

gtback commented Dec 1, 2014

Thanks, @wbolster. My other long-term goals are primarily to get rid of the "bindings" (stix.bindings) in favor of some more-Pythonic and easier-to-maintain alternative, and also adding support for Python 3. The former should make the latter much easier.

@wbolster

This comment has been minimized.

wbolster commented Dec 1, 2014

Have you considered PyXB instead of generateDS?

@wbolster

This comment has been minimized.

wbolster commented Dec 1, 2014

In the mean time, it would be nice if you could document (maybe just a README in the bindings/ directory would suffice) the process of (re)generating the bindings.

@bworrell

This comment has been minimized.

Contributor

bworrell commented Dec 1, 2014

@wbolster, we looked at PyXB when we started python-cybox development and unfortunately it wasn't able to process the CybOX schemas, so we went with generateDS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment