-
Notifications
You must be signed in to change notification settings - Fork 164
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
Dict bool keys #1771
Dict bool keys #1771
Conversation
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.
Add tests like looping and writing/reading again and again from a bool dictionary. Value can be of different types like, integer, float, string, etc.
integration_tests/test_dict_bool.py
Outdated
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.
We are doing d[True]
and d[False]
. Let's also do something like,
d[i%2 == 0] += 1
And check the count of odd and even numbers for some range [0, n)
. To make sure things work correctly for runtime boolean keys.
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.
Looks like +=
isn't currently supported for dict
yet. I have added this test with =
.
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.
No worries.
Looks like
+=
isn't currently supported fordict
yet.
Let's support this in a separate PR, once you complete this and your other open PR.
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.
Did you generate these by hand?
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.
No, I added (and later removed) a test in ./tests/tests.toml
with asr
and asr_json
options.
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.
Ah I see.Then you can remove these .stdout
and .json
files. Not needed any more.
I added (and later removed) a test in
./tests/tests.toml
Correct thing to do.
1adf567
to
330efef
Compare
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.
If llvm
test is enabled in integration_tests/CMakeLists.txt
then most of the time registering the same test in .toml
file is not needed.
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.
Delete this as well.
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 one too.
Work in progress. Towards #1710.