Skip to content
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

feat: Brought use_cats and get_nterms into the scope of the python bindings #42

Merged
merged 10 commits into from
Jun 9, 2024

Conversation

mjsutcliffe99
Copy link
Contributor

@mjsutcliffe99 mjsutcliffe99 commented Jun 6, 2024

Brought use_cats and get_nterms into the scope of the python bindings (such that these can now be changed/read within Python when using the bindings).

Moreover, the Python bindings for simplifying graphs did not work, but a couple of trivial changes seem to fix it.

@mjsutcliffe99 mjsutcliffe99 changed the title Fix some scalar discrepancies in the pybindings Brought use_cats and get_nterms into the scope of the python bindings Jun 7, 2024
Copy link
Collaborator

@ABorgna ABorgna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Make sure to run a formatter on the code, and follow the other linter recommendations.
See: https://github.com/Quantomatic/quizx/blob/master/CONTRIBUTING.md#-coding-style

Comment on lines 15 to 19
#d = json.loads(s.to_json())
#d["power2"] *= 2
#dStr = json.dumps(d)
#return PyzxScalar().from_json(dStr)
return PyzxScalar().from_json(s.to_json())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a leftover from debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially, yes. The scalar doesn't seem to be accurate (largely, it seems, because not all of the scalar information gets encoded via json) - but this is an issue I'm yet to resolve (but will likely make a separate pull request for in due course). These remarked lines of code were just part of my trying to fix that.

@ABorgna ABorgna changed the title Brought use_cats and get_nterms into the scope of the python bindings feat: Brought use_cats and get_nterms into the scope of the python bindings Jun 7, 2024
mjsutcliffe99 and others added 4 commits June 9, 2024 01:11
Trying to include use_cats here seemed to cause formatting issues which would not resolve, but it doesn't seem necessary to include it here anyway
@ABorgna
Copy link
Collaborator

ABorgna commented Jun 9, 2024

I added a couple fixes to pass the checks ran by just check (and CI).
Thanks again!

@ABorgna ABorgna merged commit 5c91ab2 into zxcalc:master Jun 9, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants