Operator Overloading
Description of issue:
The +, -, *, / operators should be operator overloaded for the basic operations in AbstractOperation, the operations to be overloaded are described in #9 (closed).
Linked Requirements:( 10 )
Potential Issues:Problem might be cyclic imports in AbstractOperation from importing core operations.
Blockers:( #9 (closed) )
Issue Requirements:- All operators in the description should be overloaded for operations.
None
Activity
marked this issue as related to #9 (closed)
changed milestone to %Sprint 1
assigned to @anglo547
created branch
10-operator-overloadingto address this issuechanged milestone to %Sprint 2
I have made a small functionality test that shows that inheriting the implementation of overloaded operators works just as intended which makes the implementation of the functionality of this issue really easy.
What remains is figuring out how to solve the cyclic import problem caused by the operator overloading functionality in the module "operation" requires to import some core operations from the module "coreoperations", but this is a problem because "coreoperations" imports "operation". A possible solution would be to use the "import x as y" syntax instead of "from x import y", however this breaks with the style of the rest of the import statements and there was some trouble when trying to fix a similiar issue with this type of solution before.
Perhaps one way to deal with this would be to implement another class in an additional module that can hold the implementation of the overloading which the "AbstractOperation" class inherits from, but maybe this doesn't solve the issue as it might just be changed to a 3 length cycle.
I will look at the "import x as y" solution for now as I don't really know what else to do to solve this.
I decide to use the "import x as y" solution since I couldn't come up with another way to do it and since it worked as intended.
Added working implementations of the overloaded operators for both Operation and Number arguments. Also add tests that verify that this works.
Edited by Angus LothianFor some reason the tests failed in the pipeline but passed locally. It has to with the "import b_asic.core_operations as core_ops" and says that the module b_asic has no attribute core_operations. Trying to find a solution for this but StackOverflow says it's caused by circular imports. Will investigate further to find out if I can get away with the "import x as y" solution somehow.
Tried another "import x as y" statement that worked fine, so it probably has to do something with the circular imports. Now another solution to the circular imports will probably have to be looked at...
Edited by Angus LothianThis thread explains that the problem can't be solved by using "import as": https://stackoverflow.com/questions/31844163/attribute-error-module-object-has-no-attribute-caused-by-circular-imports
The alternatives now are:
- Move the necessary code from "core_operations" to "operations", this would however make the module very large and not very well structured.
- Use local imports in for example the definition of add.
- Use some sort of dependency injection, which however require some sort of restructuring of how imports are handled in this project and is thus not preferred.
I have decided to attempt option number 2 for now.
mentioned in merge request !14 (merged)
Created merge request for the issue in !14 (merged)
added Needs review label and removed 1 deleted label
Closed in !14 (merged)
removed Needs review label