Strange struct behavior.

[taras....@gmail.com]

I have recently discovered that some of the struct usages in our codebase had keys containing dots (say foo.bar), which means that the only way to access it is via getattr function.

Overall, struct reminds me Python's namedtuple and its behavior in such case seems more appropriate:
>>> import collections
>>> collections.namedtuple('Foo', ['foo.bar'])
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/collections.py", line 351, in namedtuple
'alphanumeric characters and underscores: %r' % name)
ValueError: Type names and field names can only contain alphanumeric characters and underscores: 'foo.bar'

or in Python 3
>>> import collections
>>> collections.namedtuple('Foo', ['foo.bar'])
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/collections/__init__.py", line 361, in namedtuple
raise ValueError('Type names and field names must be valid '
ValueError: Type names and field names must be valid identifiers: 'foo.bar'

The only reason I can think of for allowing arbitrary keys is to allow users serialize their dictionaries into json, since struct.to_json is the only way to serialize data structures exposed to users. In such case would it make sense to just have a separate native function to_json that supports all supported data structures and stop accepting dots and possibly other types of keys like keywords (https://github.com/python/cpython/blob/master/Lib/collections/__init__.py#L357-L365)?

[Laurent Le Brun]

Good point. Can you please file the issue on GitHub?

Adding a restriction would make sense, but we should first review the use-cases (I'll check Google code base), and introduce a --incompatible flag for that change.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/7626bc49-7796-48b4-b002-2f85009d12af%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Laurent

[Taras Tsugriy]

Thank you for the supersonic response, Laurent! I'll file an issue and would be happy to help with implementation as well.

Do you happen to have an opinion as to how to_json should be exposed? Does native.to_json(val) look reasonable?

--

Best regards,
Taras Tsugrii

[Taras Tsugriy]

Filed a GH issue https://github.com/bazelbuild/bazel/issues/6095

[Laurent Le Brun]

I remember some comments about removing the "to_json" method from struct (because the name "to_json" can conflict with the user's fields). I can't find any bug for it, though.

The issue is that we don't have a nice place to put it. (there was also the idea that we could expose Bazel functions through a load() statement, e.g. load("<bazel>", "to_json") and avoid polluting the namespace).

--

Laurent

[Taras Tsugriy]

Yes, the place to put it is indeed somewhat tricky :) We haven't had an issue with to_json conflicts, but I can see how it could look somewhat arbitrary and awkward to deal with in case someone really needs a to_json field :) This may be a good time to discuss a way of exposing other built-in modules inside of the execution environment. Building on top of your suggestion, maybe something like

load("//__native:modules.bzl", "json")

json.to_json(...)

So that instead of exposing individual functions, cohesive modules focused on particular areas could be exposed. It's probably also the easiest to use some valid load strings in order to not have to special case this use-case in Label class, but overall I like your suggestion.