-
Notifications
You must be signed in to change notification settings - Fork 571
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
Added unit test for issue #977. #1112
Conversation
@dannmartens I see the discussion that lead to this PR - thanks for it - but I can't merge it in or all PRs will fail! Are you planning on fixing the code to pass this test? |
No, I currently do not have the resources to dive into this any deeper. I created this PR on request by @white-gecko: #977 (comment) This PR documents the issue, I gather it is not intended for merging at this stage. |
OK, well thanks for the PR - yes it is the very best way to document an Issue! I'll try and put some resources into it as I want all the various, niggling namespace issues gone. |
Hello, I can confirm that URN-based prefixes are still not being serialized into the .ttl format. Is this issue being looked into? I see that this has been labeled wontfix. Are there any plans to resolve this? |
@nemesamade this PR points to the issue but provides no indication as to how to solve it. Since the resulting turtle is valid - the ISBN URN is I'm not sure who is motivated to solve this one but if you would like to, we (maintainers) would be happy to review any PRs! |
@nicholascar Thanks for the status update. I might try and dig in and solve this one if I can find the time then, as it's a rather annoying inconvenience for a project I'm working on. |
@nemesamade great, well if you can get close to a solution (i.e. find all the bits) but need a hand, let me know! |
I've opened a PR (#1274) that addresses and resolves this issue by modifying namespace.py, along with adding some tests for URN namespace handling by compute_qname and turtle serialization. The change didn't break any existing tests and passes the new tests so it should be an easy fix:) |
These test should maybe be committed with @unittest.expectedFailure |
We do need a standard way of adding tests for un-solved issues but can't afford for them to fail so this does look like a solution! I'll try and add this to the PR. @aucampia if we start to add lots of tests annotated like this, is there a way to find all those tests in the future? I don't know of a unittest tool to find such tests. Would we just have to full text search all files in |
If I run the unit tests directly with
Though sadly when running with nose they don't get reported in a special way. I think there may be more reporting options but I would have to look into it more. |
Rerunning tests as this should pass due to fix mentioned above. |
Open/Close retest |
This test was created before the need to decode graph serialization results was removed, so just updating for that. Now it should pass
@nicholascar writes: If the tests here pass, it shows that
master
now addresses #977.Closes #977