Add photonlibpy #1040
Add photonlibpy #1040
Conversation
|
Do we ever check version mode strings? |
| working-directory: ./photon-lib/py | ||
| run: | | ||
| git fetch --tags --force | ||
| python setup.py sdist bdist_wheel |
There was a problem hiding this comment.
I'd recommend using pipx run build.
There was a problem hiding this comment.
pipx is currently being very sad about installing for me.... thoughts on leaving this one for later?
There was a problem hiding this comment.
Hmm, pipx's docs recommend scoop over pip for installing pipx.
There was a problem hiding this comment.
I'd probably specifically recommend https://github.com/hynek/build-and-inspect-python-package in GitHub Actions though
photon-lib/py/setup.py
Outdated
| print(f"Building version {versionStr}") | ||
|
|
||
| setup( | ||
| name='photonlibpy', |
There was a problem hiding this comment.
Consider using pyproject.toml, or setup.cfg
There was a problem hiding this comment.
Given that setup.py has expanded already to have build-time python logic.... I'm not sure I know enough to migrate it quickly? As long as you're not certain this will cause immedeate issue, I'd personally rather stick with "works" and enter an issue to followup next summer?
Stop being 2017 dean.
I am not currently.... I kinda avoided the check out of selfishness. But agreed we... probably... should keep feature parity. Granted, due to the whole pep440 version scheming thing (which either I"m still missing something on, or python & pypi's versioning requirements just don't jive with), I'm not sure it's fully applicable at the moment. Well, at least... to make it work correctly, the results of git describe have to be dumped into a source file, and then some notion of "coprocessor expected version" is checked. The version of the python library remains independent. Thoughts on whether that's value add? |
|
Well, without it, the deserialized result could just be total garbage. I think it's something we should absolutely have until we get protobuf in. |
|
|
|
Awesome! Since this doesn't touch anything else in photon beside CI, I'm good to be aggressive and merging now |
|
|
||
|
|
||
| def setVersionCheckEnabled(enabled: bool): | ||
| _VERSION_CHECK_ENABLED = enabled |
There was a problem hiding this comment.
This local variable is unused.
There was a problem hiding this comment.
smooth brain moment. issue goin in
| x = self.decodeDouble() | ||
| y = self.decodeDouble() | ||
| z = self.decodeDouble() | ||
| translation = Translation3d(x, y, z) |
There was a problem hiding this comment.
FYI we implemented getStructTopic, not currently released but it'll be available in the kickoff release. See https://github.com/robotpy/mostrobotpy/blob/main/subprojects/pyntcore/tests/test_struct_topic.py
Add a basic photonlib implementation in python.
all done