Drop support for pytest, don't try to find node when executing fails

[alexmojaki]

Based on discussion in #14 (comment)

[alexmojaki]

This has caused a loss of coverage here:

python-varname/varname.py

Lines 207 to 214 in 0a7370e

	try:
	# have to be used in a call
	assert isinstance(node, (ast.Attribute, ast.Call)), (
	"Invalid use of function `will`"
	)
	node = node.parent
	except (AssertionError, AttributeError):
	pass

Specifically the last two lines are no longer covered, i.e. no exception is ever raised.

What's going on here? It looks like you're trying to support invalid usage?

[alexmojaki]

This innocent-looking change is what causes the loss of coverage. Making only this change on master has the same effect. Something very fishy is going on and I don't really want to understand it, it looks like more things need removing.

[pwwang]

This has caused a loss of coverage here:

python-varname/varname.py

Lines 207 to 214 in 0a7370e

	try:
	# have to be used in a call
	assert isinstance(node, (ast.Attribute, ast.Call)), (
	"Invalid use of function `will`"
	)
	node = node.parent
	except (AssertionError, AttributeError):
	pass

Specifically the last two lines are no longer covered, i.e. no exception is ever raised.

What's going on here? It looks like you're trying to support invalid usage?

I should have tested if raise_exc is True/False here. If True, then I should raise the exception rather than pass.

I can take care of this, as well as the coverage.

[alexmojaki]

But you're gonna get rid of raise_exc soon, right?

Let me know when I should get back to the other PR.

[pwwang]

We agreed to get rid of var_0 etc, not raise_exc. I actually want to keep it for the cases where people intend not to raise exceptions. See if it makes sense to you for my most recent commit.

[pwwang]

If so, you can go back to the other PR.

[pwwang]

By the way, I want to put a credits section in the README to include both you and your executing project, and maybe future contributors. Do you want to find an icon for executing?

[alexmojaki]

OK, I didn't realise you were planning on returning None. I guess that's slightly better than var_0. Are you still going to make raise_exc=True the default?

I think will is implemented wrong. Consider this example:

import varname


class Foo:
    @property
    def bar(self):
        print(varname.will())
        return "abc"


Foo().bar.upper()

print(varname.will()) prints bar, not upper.

_get_node() is always going to be the node that lead to calling will, in this case the node Foo().bar. You always want the parent, and the parent should always be an attribute. The original child node can be anything, it doesn't need to be an attribute or call. I see nothing wrong with using will inside __getitem__ like in test_will_malformat, e.g. to be able to writec1[1].do() where will returns do.

Beyond that, the concept of will seems very questionable to me, even by dark magic standards. Can you show me a sensible use case? I think not using will will always be a better choice.

BTW, node.parent comes from here: https://github.com/alexmojaki/executing/blob/2f0d1c8a1ac145836c814a06bb79a62bdee7ddb1/executing/executing.py#L193-L195

So you can expect everything except the top level node to have a parent.

[pwwang]

This is the real use case for will: https://github.com/pwwang/cmdy

As the example shown in README, it works as a router. For the use of that package, I want some functionality like if there is nothing followed, the command should be executed. For example:

import cmdy

cmdy.ls()  # execute ls

Otherwise, hold it and let the "will" to handle it:

cmdy.ls().fg() # ls() will not run the command, but fg() will do the job

[pwwang]

The default of raise_exc will be kept True. I think it is better for people who use it to be aware of the invalid usage. If you want, and are aware of this, you can set it to False to suppress the exception.

It makes sense for c1[1].do() if .do() is intended to be called after c1[1].

The purpose of will is to know what will be called after the current one.
I am looking into the issue you mentioned (#17)

[alexmojaki]

OK, I can see how you need to use will to make that API work, but I'm not sure it's a good API. In particular it's going to confuse users who are used to Python not working like that. I would suggest instead of cmdy.ls().fg() they should write cmdy.fg(cmdy.ls) or cmdy.ls.fg(). Those can be implemented with less magic.

[pwwang]

All breaks (including #18 ) about will is probably due to the rush remodeling of it this morning. Let me add some more details of tests for it to make sure it works as expected.

I like this idea cmdy.ls.fg(). However, ls takes arguments (i.e. cmdy.ls(l=True) to run ls -l). My original API is like

cmdy.ls() # run the command
cmdy.ls(_hold=True) # hold the command and wait

The initial idea of will is to get rid of that _hold argument.

[pwwang]

Or if will is so special then we can choose not to expose it.

[pwwang]

Now I can confirm that if a node is ast.Call, it must have node.parent. See the abstract grammar here: https://docs.python.org/3/library/ast.html#abstract-grammar

So that I don't have to do this if I know a node is ast.Call:

try:
  node = node.parent
except AttributeError: # will never raise
  ...

[alexmojaki]

Yes, the top node should always be an ast.Module, everything else has a parent.