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

formatter design #1

Open
gnecula opened this issue Oct 12, 2015 · 6 comments
Open

formatter design #1

gnecula opened this issue Oct 12, 2015 · 6 comments

Comments

@gnecula
Copy link
Owner

gnecula commented Oct 12, 2015

I do not quite like how formatters work. Right now, they modify the observation dictionary in place. This makes it easy for the formatter, because you do not have to copy the observation, but has unpleasant consequences:

  • the side-effects in the observation are visible to downstream functions, but not to filter and ignore functions, which are processed before
  • the side-effects make it impossible to write one-line formatters using 'lambda' (but that is really a weakness in Python). Even without side-effects it would be hard to write one-liners.

What I would really like is to specify places in the observation where I want to make some changes. For example, I want to split the lines for key1[].foo, or I want to replace all strings that match a date in key2.dates[].

@gnecula
Copy link
Owner Author

gnecula commented Oct 12, 2015

Also, I am proposing that only agents specify formatters. The spy function does not really need them because you can just as well format the value before passing it to spy. The spy_point should not have it because we want to discourage writing bond-specific code in production code.

@xkrogen
Copy link
Collaborator

xkrogen commented Oct 12, 2015

Agree that I don't really like the way they work right now. I'm sure we can find something better.

As for formatters in spy_point... I can imagine a situation where being able to specify a formatter right next to the function could be useful, e.g. when you know that there's one argument you'll never want to observe, or something along those lines. I agree that minimizing bond code in production is important but it's something to consider.

@gnecula
Copy link
Owner Author

gnecula commented Oct 12, 2015

For now I have moved the formatter than I had in bond_reconcile._print to the testing code.

@gnecula
Copy link
Owner Author

gnecula commented Oct 12, 2015

Here is a proposal for helping the writing of formatters, without writing a new JSON serializer. Essentially we specify a list of pairs (location_regexp, action) where the location_regexp says what keys in the observation should be acted on, with the keys being of the form "key1.key2[xxx]", and the actions being things like "split_lines", "search_replace(regexp, replacement)"

So we can write things like:

formatter=formatters.split_lines().at('.*\.what$'), 
...
formatter=formatters.search_replace('random=\d+', 'random=...').at('.*\.what\[\d+\]$')

This allows one to write simple formatters without defining auxiliary functions. Another advantage is that most of the time we use the standard JSON formatter and we do not need to re-traverse the object.

This notation should allow one to write new actions.

Using regexp to match keys is fast, but the notation is awkward because the key language contains "." and "[ ]" which have special meaning as regular expressions.

@xkrogen
Copy link
Collaborator

xkrogen commented Oct 12, 2015

Yeah, I like that direction. We could probably allow regex for full flexibility, but also provide easier ways, e.g. split_lines().atKey('what') would achieve the same thing as your first line and search_replace(...).atKeyWithValue('what', '\d+') would achieve the same as the second line (or something like this - generally I just I feel that most of the time full regex would be unnecessary and a simpler interface would be nice)

@gnecula
Copy link
Owner Author

gnecula commented Oct 15, 2015

Another useful formatter would be a rounding function for floating point values.

xkrogen added a commit that referenced this issue Feb 18, 2016
…ybond. Could use improvement, as discussed in issue #1.
xkrogen added a commit that referenced this issue Feb 25, 2016
…ybond. Could use improvement, as discussed in issue #1. Additionally fix a small bug when trying to use agent filters, e.g. startswith, on fields which are string-like but not strings.
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