Added class NodeList

[jacobsvante]

Allows for basic attribute filtering using filter_by method. The user-facing API is inspired by the SQLAlchemy Query object. Usage looks like this:

root.resources = create_resources(root.raml_obj, NodeList(), root,
                                  parent=None)
post_resources = root.resources.filter_by(method='post')
image_resource = post_resources.filter_by(name='/image/').one()

The only place I'm using it in the existing codebase is in the call to create_resources in ramlfications.parser.parse_raml. But if you like the idea I'll gladly put it in other places it fits (which is basically any place where a list is used).

[econchick]

Ah very interesting! I'll look at this tomorrow and may take a little time to think about this feature itself.

(If you're a lurker and have an opinion about this feature, let's hear it!)

[jacobsvante]

I think most lists have been replaced by NodeList objects now. If I missed something please do tell!

I also added support for filtering children that are instances of dict or its subclasses. E.g:

>>> mydicts = [dict(winner=True), dict(winner=False)]
>>> mydicts.filter_by(winner=True).one()
dict(winner=True)

Btw! You could probably make use of .one() instead of doing listobject[0] everywhere in the code. Haven't had the energy to look at it though :stuck_out_tongue_winking_eye:

[econchick]

Hey @jmagnusson - wow super sorry for the delay!!

I really like this feature. I'm wondering if you would be able to rebase this onto master in the next couple of days? I'd like to release this within ramlfications v0.2.0.

If you can't get to it soon (and why should you, it's the holidays), let me know and I can try rebasing/merging myself.

[jacobsvante]

No worries! I've rebased the PR now. I also added a new commit that adds py35 to the list of test envs. When I did I noticed that there was a problem with the calls to assert_called_once raising an AttributeError. So I dug a bit further and it seems to be a buggy method that doesn't always work as it should. I replaced these calls with assert mock_obj.call_count == 1 which uncovered that some of the methods weren't actually being called once, as the method name would suggest. Perhaps you could look into this?

See this commit for more info: https://github.com/jmagnusson/ramlfications/commit/2955b5a5958f9bb70e7b613e33cbc6834f428eda

Thanks,

[jacobsvante]

Any chance you could look at the assert_called_once issue? Would be nice to get this merged before too many new changes are made :sweat_smile:

[econchick]

@jmagnusson ah thanks for the gentle nudge :) I will today when I get into the office!

[econchick]

Hey @jmagnusson - finally looked into it. FYI I just ran it on my own branch, I haven't tried it on your PR.

I remember having this issue when the mock library was updated, I forget which version but it was one that was after 1.0.1. And I forget why, but mock_obj.assert_called_once no longer worked. So rather than just update the code with assert mock_obj.call_count == 1, I just pinned an older mock version. But I'm fine with this PR changing that with a later version of the mock library.

The reason that you get some failing tests is probably because requests isn't installed. Can you pip install requests to see if you get passing tests? The requests dependency is optional, but encouraged for Python versions older than 2.7.9 and 3.4.3 (and can be installed via pip install ramlfications[all]). I should probably add that as a test dependency :D Perhaps that's why it's failing on Travis. If you want to add it to the PR, I'd put it both in tox-requirements.txt and setup.py under tests_require.

This also makes me realize that in ramlfications/utils.py, I currently have a hard pin of Python version rather than a >=. I should fix that too :)

With regards to your PR, I think I would like you to break this up/condense this into 3 commits: one with adding py35 test env, one with fixing the mock lib & related tests, and finally one with your feature. Would that be too much?

[codecov-io]

Current coverage is 96.96%

Merging #52 into master will decrease coverage by -0.73% as of 12b4526

Powered by Codecov. Updated on successful CI builds.

[jacobsvante]

Thanks for looking at this.

Installing requests did not make the tests work for py35.

I removed py35 from tests and also reverted to mock==1.0.1 (all newer ones fail on assert_called_once), created a ramlfications._compat module and moved some compatibility logic that was repeated. Also rebased into one commit to make it easier to overview.

Regarding the mock/py35 issues I would feel a bit more at ease if you would look at them as you know the codebase a bit better than me :sweat_smile:

[econchick]

OK so I'm ready for this (super sorry for the delay). But I'm not sure if I want to put it in master, as this is a pretty big feature relative to the package. I would like to release it in 0.2.0, but a lot of the codebase has changed. There are a couple of options:

1. You rebase on the v0.2.0-dev branch - which will require some re-tooling since I simplified the parser code.
2. I pull it in myself, and I do the retooling. I will try to preserve the commit history, but not sure if I can (I'm totally not a git-ninja, but I assume git cherry-pick from your branch to the new branch would work here). I'm more than happy to do the leg work, especially since I didn't get this merged sooner (aka it's kind of my fault haha).

Either way, you would definitely be on the AUTHORS.rst and I'd call you out on the changelog.

[jacobsvante]

It was less than half an hour of work so it's all good 😁It was much easier to integrate now with the refactoring! Added some additional tests to make sure that the list of nodes returned by ramlfications are instances of NodeList.

See #93 for the new PR.