fix: Make filtering children by class in traverse() actually work

[asb]

The traverse() utility function was previously written to filter
children by class using issubclass(child, klass):. But the first
argument to issubclass() must be a class so this will always raise an
exception when trying to use the functionality. This patch corrects the
call to isinstance(child, klass) and adds a test.

[pbodnar]

@asb, thank you for the fix. I guess it was a mistake to accept the previous PR without a more complete set of tests.

If you don't mind, I have just slightly changed the PR title - can you please do the same with your commit message, while amending the commit with my little suggestion from the code review? :)

[asb]

@pbodnar: thanks for such a rapid review, I've made the suggested changes.

It's an unfortunate bug, but the traverse utility does seem pretty handy to have in the codebase. It's nice to have a helper to make it easier to do information extraction or even light modifications at the AST level, as an alternative/companion to customising the renderer.

[pbodnar]

@asb, thanks for the quick fix. :)

[asb]

I'm now wondering what the behaviour should be when specifying a class to filter on and also setting include_source=True. As it is, the 'source' node will be yielded even if it doesn't match the filter. Perhaps it doesn't matter much either way, but this seems a little surprising to me. It also doesn't seem too important one way or another though.

Given filtering by class didn't work previously, there's an opportunity to tweak this without breaking anyone's code.

[pbodnar]

@asb, good point. I can imagine that most people would expect, like you, that filtering by class will always win over include_source=True. In that case, to follow the principle of least surprise, we should probably change the current behavior. Are you willing to do the change yourself (change impl. + doc + unit test)?

[asb]

Sure thing - see #158