Changes to support CDM data import #1
Conversation
jeff-cohere
added
enhancement
| - name: Running tests (${{ matrix.os }}) | ||
| run: coverage run -m unittest discover | ||
| env: | ||
| DTS_KBASE_DEV_TOKEN: ${{ secrets.DTS_KBASE_DEV_TOKEN }} |
There was a problem hiding this comment.
there should already be some kbase dev tokens in the environment if you check in the settings for this repo.
There was a problem hiding this comment.
I tried KBASE_CI_TOKEN, but it doesn't work. The GitHub documentation is entering the Enterprise Software Heat Death stage of its lifecycle, so it's hard to figure out the difference between org secrets and repo secrets, or even whether anyone at GitHub knows what the difference is supposed to be. So I'm reverting my change here for the time being. I'll revisit when we have this other stuff sorted.
| """ | ||
| `client.deleteTransfer(id) -> None | ||
| `client.cancel_transfer(id) -> None |
There was a problem hiding this comment.
it's probably worth using one of the recognised python documentation formats -- depending on what IDE you use, you may be able to automatically generate documentation skeletons for functions and if you add a linter, you won't get a bunch of errors because the docs don't follow pydocstyle.
There was a problem hiding this comment.
Yeah, I've been hesitant to strap myself all the way into Python dev mode, but it's probably a good time to address some of this stuff. I don't use an IDE, though, because I find they make it easier to produce mountains of rubbish with almost no effort.
"one of the recognised python documentation formats" gets at the key issue -- there's 10M ways to do everything in Python. In this it takes after my other "favorite" language, C++, in which everyone strives to prove how clever they are by trotting out esoterically cute ways of doing trivial things that don't pertain to any other language.
There was a problem hiding this comment.
FWIW I use the sphinx documentation style, and ruff for python code linting/formatting. I use VSCode for working with python -- there's an extension for generating python documentation for functions and a nice ruff integration, so you can handle all that stuff whilst you're working on the code. I'm sure that there are also emacs / vi / etc. integrations for ruff, too.
| import os | ||
| import unittest | ||
|
|
||
| class TestClient(unittest.TestCase): |
There was a problem hiding this comment.
I would switch to using pytest for tests -- it is a lot more flexible that unittest as you can easily parametrise your tests (run the same test on a bunch of different inputs) and the test output reporting is much more helpful.
There was a problem hiding this comment.
Ah, yes, one of the 10M testing packages. Thanks for the recommendation!
There was a problem hiding this comment.
I made an issue for this.
| self.assertTrue(client.uri) | ||
| self.assertTrue(client.name) | ||
| self.assertTrue(client.version) |
There was a problem hiding this comment.
these aren't really worthwhile tests -- maybe they can be improved when you move to pytest, but I would expect the tests to check the values of these attributes, not just whether they are truthy or falsy.
There was a problem hiding this comment.
Yeah, I can definitely address these when we move to pytest.
There was a problem hiding this comment.
pytest uses assert so these self.assertTrue and self.assertFalse statements would turn into something like:
assert client.uri == "some_client_uri"
assert client.name
assert isinstance(client.version, str)
assert len(client.name) > 15
This PR pulls in changes made to support our ad-hoc ("heroic?") import of data into CDM.
This is a pretty new repo, so I don't have anything CI-ish in place yet. @ialarmedalien , we were talking about getting dtspy "up to spec" with some GitHub Actions, style checks, and what-not. Maybe we can use this PR to get things in shape. Please feel free to invite whatever other reviewer(s) you think would be good representatives of the KBase Python Way.
Right now, we just use a basic
pip -r requirements.txtinstall process. I'd prefer not to wander into the bog of what passes for Python package management tools, since they're all horrible. :-)Update
I implemented some unit tests, though only for search and metadata features (for now).