Constructor update #213
Constructor update #213
Conversation
|
Just updated #212. Please update as needed. |
* Update all usages
* We can now run ruff check . from root * Fix imports unused, unsorted in tests * Fix fstring errors, unused variables, etc * All test changes via ruff check . --fix
* Only worked as copy constructor before so explicit copy replaces
* Push to_tensor to all callers
* Basically just keep imports reasonable and avoid logic errors (unused variables etc)
ntjohnson1
force-pushed
the
constructor_update
branch
from
038baa5 to
2655c82
Compare
…oject no longer matches pylint
|
Great. I rebased and did a quick check so this should be ready to go now. I elaborated a bit more on the PR description in case that is useful, but feel free to let me know if anything seems weird or I misinterpreted the direction we were discussing for the interfaces. |
|
I notice differences from Can you align the |
* Remove undocumented methods/members * No longer need to explicitly exclude members by name * Move all tensor types to use slots * Don't skip __init__ since we were dropping that entire doc string
* Merges __init__ with class definition instead of a separate __init__ entry
I added two commits for this since there are some stylistic preferences (details are in the commits but adding more info in case it is clearer). For the first I dropped In the second commit I merged the Since this brings all our tensor types into the same format in the future copy pasting should make new types match as well. |
* tensor Update __init__ and remove from_data: * Update all usages * tensor add explicit copy and deepcopy dunder * sptensor update __init__ and remove from_data * sptensor add copy and deepcopy dunder * ttensor: Rename u to factor_matrices to match ktensor. * ruff: Add minimal coverage to tests dir: * We can now run ruff check . from root * Fix imports unused, unsorted in tests * Fix fstring errors, unused variables, etc * All test changes via ruff check . --fix * ttensor: update __init__ and remove from_data * ttensor: add copy and dunder deepcopy * ttensor: Remove from_tensor_type * Only worked as copy constructor before so explicit copy replaces * tensor: Remove from_tensor_type: * Push to_tensor to all callers * ruff: Add bugbear to catch bugs and better align with pylint * pylint: Disable false positive if we don't remove pylint first * sptensor: Remove from_tensor and push conversion to callers * ttensor: Add to_tensor * tutorials: Update to use new interfaces * Updated doc and test to capture or ruff support of tests directory: * Basically just keep imports reasonable and avoid logic errors (unused variables etc) * Missed a pylint disable statement in the rebase. A search over the project no longer matches pylint * Documentation consistency: * Remove undocumented methods/members * No longer need to explicitly exclude members by name * Move all tensor types to use slots * Don't skip __init__ since we were dropping that entire doc string * Documentation style: * Merges __init__ with class definition instead of a separate __init__ entry * Add forgotten arguments to docstring
* A valid indices check added to the sptensor.from_data() class method: if an index is found that is less than zero or gte to the size of the dimension it indexes, a ValueError is raised. * Formatting * using tt_subscheck and shape check in from_data * formatting the file. * Making a check for when subs.size == 0, because this means that tt_subscheck() returns True, meaning empty subscript arrays are considered valid in the sptensor constructor. * Constructor update (#213) * tensor Update __init__ and remove from_data: * Update all usages * tensor add explicit copy and deepcopy dunder * sptensor update __init__ and remove from_data * sptensor add copy and deepcopy dunder * ttensor: Rename u to factor_matrices to match ktensor. * ruff: Add minimal coverage to tests dir: * We can now run ruff check . from root * Fix imports unused, unsorted in tests * Fix fstring errors, unused variables, etc * All test changes via ruff check . --fix * ttensor: update __init__ and remove from_data * ttensor: add copy and dunder deepcopy * ttensor: Remove from_tensor_type * Only worked as copy constructor before so explicit copy replaces * tensor: Remove from_tensor_type: * Push to_tensor to all callers * ruff: Add bugbear to catch bugs and better align with pylint * pylint: Disable false positive if we don't remove pylint first * sptensor: Remove from_tensor and push conversion to callers * ttensor: Add to_tensor * tutorials: Update to use new interfaces * Updated doc and test to capture or ruff support of tests directory: * Basically just keep imports reasonable and avoid logic errors (unused variables etc) * Missed a pylint disable statement in the rebase. A search over the project no longer matches pylint * Documentation consistency: * Remove undocumented methods/members * No longer need to explicitly exclude members by name * Move all tensor types to use slots * Don't skip __init__ since we were dropping that entire doc string * Documentation style: * Merges __init__ with class definition instead of a separate __init__ entry * Add forgotten arguments to docstring * Sptensor: Add better validation check of shape consistency * Added as an assert so `python -O` can run to skip it if desired * Tensor: Docstring fails on my machine because it prints dtype int64 * TTensor: Catch uncovered line from previous PR * Black: Run black and update local test to check black to avoid hitting the error in CI. --------- Co-authored-by: DeepBlockDeepak <[email protected]> Co-authored-by: Danny Dunlavy <[email protected]> Co-authored-by: Nick <[email protected]>
Addresses #187 #188 (and ttensor but I didn't bother with a ticket for that) and #209. Filed #214 as follow up because tenmat's relationships between the constructors is kind of weird, so needs more thought. I followed the design discussed in #145 which I believe was the intention of the first two issues.
There were also three commits ca9629b, 862201b, 2655c82 related to simplifying some edge cases in ruff. Basically, we replaced isort with ruff but weren't applying the import sorting/removal of redundant imports to our tests. Also ruff can detect and fix unused variables etc which could be useful if we realize our tests aren't actually doing what we expect.
A few notes for where I set copy=False already. I didn't look at our profiling at all to see if this helped in our performance. I didn't do an exhaustive analysis for where we can skip the copies. Generally I looked for obvious usages where copies were redundant:
While the content is mostly unrelated I expect this to clash with #212 but they both are based off latest master so feel free to merge whichever you prefer first and I'll resolve the conflicts.(Resolved)📚 Documentation preview 📚: https://pyttb--213.org.readthedocs.build/en/213/