Adding GitImplementation of Ground and tests #4
Conversation
There was a problem hiding this comment.
All of these files shouldn't be in the home directory. They should be organized under the python directory in this repo.
I think we need to chat about this a little more. I don't see anything in the code that uses the commit method or the git library imported at the top. Everything is being stored in memory, which defeats the purpose of using git. Let me know if I'm misreading how this works.
ground.py
Outdated
| from shutil import copyfile | ||
|
|
||
|
|
||
| class Node: |
There was a problem hiding this comment.
All these classes are already implemented in python/ground/common/model. We shouldn't duplicate them.
ground.py
Outdated
| if (nodeId in self.graph.nodes): | ||
| node = self.graph.nodes[nodeId] | ||
| nodeVersion = NodeVersion(node, reference, referenceParameters, tags, structureVersionId, parentIds) | ||
| # else: |
There was a problem hiding this comment.
This is Jarvis specific stuff. We should remove this.
ground.py
Outdated
|
|
||
| ### EDGES ### | ||
|
|
||
| def createEdge(self, sourceKey, fromNodeId, toNodeId, name="null", tags=None): |
There was a problem hiding this comment.
Maybe I'm missing something, but none of these methods are actually using git commands. We're returning heap objects which won't persist across multiple API calls.
ground.py
Outdated
| def getLineageGraphVersion(self, lineageGraphVersionId): | ||
| return self.graph.lineageGraphVersions[lineageGraphVersionId] | ||
|
|
||
| def commit(self): |
There was a problem hiding this comment.
I also don't see this commit method used anywhere.
python/ground_git.py
Outdated
| from shutil import copyfile | ||
| # noinspection PyUnresolvedReferences | ||
| from ground.common.model.core.node import Node | ||
| # noinspection PyUnresolvedReferences |
There was a problem hiding this comment.
Those are just for my IDE, PyCharm, so it doesn't stop a run for errors. For whatever reason, PyCharm doesn't recognize the importing of local packages, so it will show an error that is not really there. I can remove those if that is preferable
There was a problem hiding this comment.
Yeah, please remove them from the code.
python/ground_git.py
Outdated
| """ | ||
| Abstract class: do not instantiate | ||
| """ | ||
|
|
python/ground_git.py
Outdated
|
|
||
| class GitImplementation(GroundAPI): | ||
| def __init__(self): | ||
| #self.nodes = {} |
There was a problem hiding this comment.
If this is unnecessary, we should remove.
python/ground_git.py
Outdated
| def _gen_id(self): | ||
| with open(self.path + 'ids.json', 'r') as f: | ||
| ids = json.loads(f.read()) | ||
| newid = len(ids) |
There was a problem hiding this comment.
Why can't we just store an integer?
python/ground_git.py
Outdated
| f2.write(json.dumps(ids)) | ||
| return newid | ||
|
|
||
| def _write_files(self, id, body): |
There was a problem hiding this comment.
Why not reuse this fn in _gen_id?
| return str(output, 'UTF-8') | ||
|
|
||
| def _check_init(self): | ||
| if(not self.initialized): |
There was a problem hiding this comment.
What happens if I created a repo a long time ago and am reusing it now? How do I know that it's been initialized?
There was a problem hiding this comment.
the .init() method will automatically check for previous initialization and will call init again which is safe but will not replace the repo and your existing work. The ids will carry on with the highest id and continue, not replacing older files. However, this implementation does not handle the case where the folder has been previously manually created, then initialized to some other repo with previous commits that have no relation to the ground objects, it will just continue to commit to that initial repo.
There was a problem hiding this comment.
That's fine for now. Let's add a README.md to the directory with documentation like this.
python/ground_git.py
Outdated
| body["toNodeId"] = toNodeId | ||
| edge = Edge(body) | ||
| edgeId = edge.get_id() | ||
| #self.edges[sourceKey] = edge |
There was a problem hiding this comment.
Nit: Cleanup commented out code.
python/ground_git.py
Outdated
| def createNode(self, sourceKey, name="null", tags=None): | ||
| self._check_init() | ||
| if not self._find_file(sourceKey, Node.__name__): | ||
| body = self._create_item(Node.__name__, sourceKey, name, tags) |
There was a problem hiding this comment.
We seem to be reusing the class names a lot here. Not necessarily a bad thing, but can we think of a more efficient solution?
python/ground_git.py
Outdated
| self._check_init() | ||
| return self._read_version(lineageGraphVersionId, LineageGraphVersion.__name__) | ||
|
|
||
| """ |
There was a problem hiding this comment.
Same nit about commented code.
python/ground_git.py
Outdated
| os.chdir('../' + str(int(prevDir) + 1)) | ||
| """ | ||
|
|
||
| class GroundImplementation(GroundAPI): |
There was a problem hiding this comment.
Holdover from the file in jarvis, unnecessary and will delete
…riables for class names, and fixed the latest id method
Added the ground.py file, originally from https://github.com/ucbrise/jarvis, now with all datatypes implemented and all the GitImplementation methods completed, as well as adding unimplemented methods for the GroundAPI abstract class, and making some minor fixes along the way. Also, the testing file test_ground.py was added, which can be run from the command line with python 2 as 'python test_ground.py'. It contains comprehensive testing for the datatypes and GitImplementation.