-
Notifications
You must be signed in to change notification settings - Fork 8
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
C handle init #76
Merged
Merged
C handle init #76
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
679bd66
add names to some of the core tests in case the brokers haven't been …
phlptp 66a258f
fix test3
phlptp d6847b4
Merge pull request #41 from GMLC-TDC/update_test_core_names
kdheepak 23a6d2c
Merge branch 'main' of github.com:GMLC-TDC/pyhelics into develop
afisher1 9de0708
Correct copy-paste error for helicsTranslatorGetName
trevorhardy a2bc598
Merge pull request #68 from GMLC-TDC/translatorName
trevorhardy 6c95a5c
Add super __init__() for select classes
trevorhardy 4172f01
Add missing ")"
trevorhardy bb8f737
Add additional fix for `helicsQueryBufferFill()`
trevorhardy 14c1e77
Test change to Python3 version of `super()`
trevorhardy 8c0e639
Fix copy-paste error for inits
trevorhardy 5fa9f76
Fix another copy-paste error
trevorhardy 7f30f42
Update helics/capi.py to use Py v3 super()
trevorhardy ce8cf15
Update helics/capi.py to use Py v3 super()
trevorhardy 3f77ed4
Update helics/capi.py to use Py v3 super()
trevorhardy 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
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.
Is HELICS expecting a utf-8 encoded string?
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 is what Mark Eberlein put in and it works so I guess, "yes"? It might make more sense to choose
ascii
. Let me know if you want me to change something.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.
It looks like the test code in HELICS is just shoving a regular
char *
into the query buffer, which seems to imply that any encoding is possible? I think this function should have an argument that specifies the encoding to use (or no encoding/raw bytes). @phlptp?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.
@phlptp; any input on what you think we should do here? I don't have a philosophical problem with supporting data structures (raw bytes) as query responses if there's a need to do so BUT I suspect restricting this to ASCII or UTF-8 will make it easier to maintain the code and easier for users to understand what they are doing. Off the top of my head, I can't think of a use case where it would be important to support a raw bytes data structure where something like JSON wouldn't also be sufficient.
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.
I think I would be fine restricting it in python. In the lower levels there are a few use cases for queries/commands getting raw binary data that I want to leave open, but that would be really complicated to use in python so don't really think we would need to support it, and if someone wanted and had the ability they could directly access the library calls.