-
Notifications
You must be signed in to change notification settings - Fork 12
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
SkeletonClient changes, mainly simplification of output formats #265
Merged
Merged
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
c25022b
Compressed SWC format option.
kebwi dae1c34
lint/ruff corrections
kebwi cd7227e
Bulk skeleton generation.
kebwi 106113d
New skeleton cache query endpoint
kebwi f94bd9e
Ruff corrections
kebwi a44d647
Trivial change
kebwi 1c5c2ac
Removed debugging code. Labeled protected functions.
kebwi 7eae57a
Ruff correctdions.
kebwi b8cb070
Ruff corrections.
kebwi 7d68e4d
Ruff changes that make the code definitively HARDER to read. Is this …
kebwi 87c1d36
Converted print statements to logging calls.
kebwi 3e936d9
Server version checking for new API endpoints.
kebwi e69e249
Ruff conformity.
kebwi 0c8f286
Added SkeletonClient.skeletons_exist().
kebwi 6a9bcd2
Ruff changes.
kebwi 4696006
Skeleton Client documentation
kebwi dd343d8
Merge branch 'master' into skeleton_dev3
kebwi 83951de
update test docs to note confusing local server issue
ceesem 0a89893
generate_bulk_skeletons_async() now reports an upper bound estimate o…
kebwi 581141d
Documentation
kebwi 1053f53
Cleanup
kebwi 95f1c8e
Versions endpoint
kebwi 3ed80d5
Update changelog
ceesem df213c0
merging
kebwi 5dd723f
SkeletonClient now only accepts 'dict' and 'swc' format requests.
kebwi 3e56070
ruff cleanup
kebwi 7bb6e10
ruff cleanup
kebwi 7d78339
ruff cleanup
kebwi 1575074
Various handling of the new 'flatdict' skeleton format.
kebwi 8159d95
ruff compliance
kebwi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should raise a more informative exception telling the user to contact their system administrator to ask them to upgrade their skeleton service to at least 0.6.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fcollman But this is a genuine assertion, a verification that the code is performing correctly. The function only receives "dict" and "swc" as inputs (see the function signature). Then on line 475, "dict" is translated to one of two internal formats ("jsoncompressed" or "flatdict") based on the server version. Later on, at the line you indicated in your comment, I am merely confirming (asserting) that the internal format has been assigned correctly based on the server version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fcollman To clarify, the API, from the user's perspective outside CAVEclient, doesn't offer "flatdict" as an option. They user asks for "dict" or "swc". Those are their options. These are translated to internal formats that communicate with the server concisely to indicate which format the server should deliver. More to the point, the server really only delivers one kind of dict at at time, but it depends on which version of the server is deployed. There is no "choosing" between dictionary formats from the CAVEclient's perspective (much less from the user's perspective, removed another level of abstraction outside CAVEclient). So, this is an assertion that the code is translating the API format to the correct internal format based on which server version is available.
As for warning the user that their server is out of date, that is handled by other code earlier in the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it! make sense didn't read carefully enough