Add snoop as potential run option

[winniederidder]

Adds features discussed in dodona-edu/papyros#109 (comment)
Extra:
run and run_async accept arbitrary kwargs which are passed to execute, which can then be passed to e.g. exec_snoop to set Snoop config values.

I wasn't entirely sure what to do with the internal directory updates you did in futurecoder. Is this something we want in general, to also trace our inner workings? I don't see how I would want that for example in Papyros, or is that needed to be able to trace the methods in the user's file?

[coveralls]

Pull Request Test Coverage Report for Build 2157877032

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 2090472257:	0.0%
Covered Lines:	212
Relevant Lines:	212

---

💛 - Coveralls

[alexmojaki]

Thanks. Instead of just passing through kwargs, I think there should be an optional snoop_config argument, and that should be used to populate the snoop Config so that things other than color can be changed.

[winniederidder]

Thanks. Instead of just passing through kwargs, I think there should be an optional snoop_config argument, and that should be used to populate the snoop Config so that things other than color can be changed.

I added a method snoop_config for this in the runner, which hopefully provides this flexibility then.

[alexmojaki]

How is snoop_config meant to be used? By overriding it in a subclass? Are users expected to call it? When does the default in exec_snoop get used?

[winniederidder]

How is snoop_config meant to be used? By overriding it in a subclass? Are users expected to call it? When does the default in exec_snoop get used?

Subclasses can indeed override snoop_config to tune it to their needs. They shouldn't need to call it themselves unless they also override execute in this case. The default in exec_snoop is currently not used, so that could be removed I suppose.

[alexmojaki]

How is snoop_config meant to be used? By overriding it in a subclass? Are users expected to call it? When does the default in exec_snoop get used?

Subclasses can indeed override snoop_config to tune it to their needs. They shouldn't need to call it themselves unless they also override execute in this case. The default in exec_snoop is currently not used, so that could be removed I suppose.

One problem with this API is that it's not easy to extend the default behaviour with a small tweak. You pretty much have to copy the code from the original method. Snoop's Config doesn't support copying or incremental changes.

[winniederidder]

How is snoop_config meant to be used? By overriding it in a subclass? Are users expected to call it? When does the default in exec_snoop get used?

Subclasses can indeed override snoop_config to tune it to their needs. They shouldn't need to call it themselves unless they also override execute in this case. The default in exec_snoop is currently not used, so that could be removed I suppose.

One problem with this API is that it's not easy to extend the default behaviour with a small tweak. You pretty much have to copy the code from the original method. Snoop's Config doesn't support copying or incremental changes.

What do you propose then? Something similar to the original approach using kwargs?

[alexmojaki]

Thanks. Instead of just passing through kwargs, I think there should be an optional snoop_config argument, and that should be used to populate the snoop Config so that things other than color can be changed.

To clarify, I meant that you'd call run(..., mode="snoop", snoop_config=dict(color=True)).

(color should be False by default, it uses ANSI codes)

[alexmojaki]

I wasn't entirely sure what to do with the internal directory updates you did in futurecoder. Is this something we want in general, to also trace our inner workings? I don't see how I would want that for example in Papyros, or is that needed to be able to trace the methods in the user's file?

On the contrary, it means that those internal directories shouldn't be traced. If you remove those lines, you can see a test failure with extra output from snoop tracing code from python_runner.

Thanks for the PR. I'm happy to merge, are you?

[winniederidder]

I wasn't entirely sure what to do with the internal directory updates you did in futurecoder. Is this something we want in general, to also trace our inner workings? I don't see how I would want that for example in Papyros, or is that needed to be able to trace the methods in the user's file?

On the contrary, it means that those internal directories shouldn't be traced. If you remove those lines, you can see a test failure with extra output from snoop tracing code from python_runner.

Thanks for the PR. I'm happy to merge, are you?

Good to know. Then I believe this is good to go! Could you then also create a new release?

[alexmojaki]

Released 0.6.0

[coveralls]

Pull Request Test Coverage Report for Build 2157877032

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 43 of 43 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 2090472257:	0.0%
Covered Lines:	212
Relevant Lines:	212

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 2157104796

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 38 of 42 (90.48%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-1.9%) to 98.148%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
python_runner/snoop.py	29	33	87.88%

Totals
Change from base Build 2090472257:	-1.9%
Covered Lines:	212
Relevant Lines:	216

---

💛 - Coveralls