-
Notifications
You must be signed in to change notification settings - Fork 6
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
Eliminate type parameters from abstract types and add fullbasis function #38
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
===========================================
+ Coverage 17.08% 34.25% +17.16%
===========================================
Files 12 12
Lines 398 400 +2
===========================================
+ Hits 68 137 +69
+ Misses 330 263 -67 ☔ View full report in Codecov by Sentry. |
Thanks! I will try to find some time to look into this work. A minor nit: if you are moving around a lot of code without modifying it, it helps a lot to have the moving and the editing in separate commits -- the review is much faster as the reviewer has an easier time focusing on the actual edits without wasting time on verifying that code is simply moved. No need to do it here after the fact, just a heads up for next time. |
The reorganization is the first commit here, which is also alone in PR in #37, since I figured you would see that first after the review request, then I would request review here only after that had been merged. If this is too confusing, let me know whatever is easiest for you! I admit I'm not so adept at the github interface since I usually use magit in emacs 😅 |
bf5f454
to
dd38fae
Compare
Converting to a draft for now since I just realized methods in |
Closing in favor of #42 |
Following on discussion in #26 and #34, this is the first step towards improving the api around basis, by eliminating the type parameters from the abstract quantum object types and adding
fullbasis
. This will require minimal changes toQuantumOpticsBase
because the currently necessary type parameters can be reintroduced when subtyping the abstract types.PS, this PR is already on top of #37.