Conversation
dosumis
left a comment
dosumis
left a comment
There was a problem hiding this comment.
Looks great. Some small changes requested. Would like to get one or two more reviews @anitacaron @udp @hkir-dev would you have time to take a look? (this is to fulfill functional spec in #1 )
|
Are we going to use poetry? |
@anitacaron - Seems reasonable, but maybe not needed on this PR - can you create a ticket & link to the details? |
| @@ -0,0 +1,71 @@ | |||
| from ..config import default_config | |||
| from typing import List | |||
| from SPARQLWrapper import SPARQLWrapper, JSON | |||
There was a problem hiding this comment.
Consider using OAK as a wrapper for querying UberGraph. There are some advantages - e.g. support for standard prefixes. @anitacaron can tell you more.
There was a problem hiding this comment.
Even thought I'm not sure how the prefix validation will be done, do we really need an excavator for a task that can be done with a small shovel?
There was a problem hiding this comment.
I know what you mean - but also thinking about potentially calling other OAK functions in future. I think worth discussing with @anitacaron.
|
Add requests.txt with dependencies at root |
We're going to use poetry for this. #7 |
|
In general project structure looks great. Can we make automatic code coverage tests part of CI? Recently I tried Codecov. It's free for public repos. You can configure it to run as part of your CI github action (example action). It will automatically calculate the code coverage change for each pull request and make a comment (such as). Also it would be wonderful if we can find similar apps for code style and quality checks. |
I have created an issue for this, #8 . |
I'll set up poetry with flask8 and add GH Action to check each PR. |
|
@ubyndr, qc didn't pass 😬 let me know if you don't get the suggestion message. Use |
Thanks for the commits, but I would prefer the qc and poetry additions in an other PR. It would be better, wouldn't it? |
|
The sooner you can start using QC, the better. As you can see, the QC is already not passing. |
Fair point, but it is not in the scope of #4 . I made some updates but ignoring |
4 participants
Resolves #4
I have added classes and methods, and empty test classes.
I'm not 100% sure about the curie validation part, class hierarchy and static methods I've added. It would be nice to have a discussion.