search

sandialabs / pyttb

Python Tensor Toolbox
https://pyttb.readthedocs.io
BSD 2-Clause "Simplified" License
26 stars 13 forks source link

Mapping of pyttb ttb functionality #291

Closed jeremy-myers closed 7 months ago

jeremy-myers commented 11 months ago

Addresses https://github.com/sandialabs/pyttb/issues/180

Summary: this PR came from a desire to ease the transition for MATLAB TTB users to pyttb. The main goal of mapping TTB and pyttb functionality is to document where naming conventions, method and data member usages, etc. differ between implementations.

📚 Documentation preview 📚: https://pyttb--291.org.readthedocs.build/en/291/

jeremy-myers commented 11 months ago

This PR fails regression tests, but only documentation was updated (no source code).

ntjohnson1 commented 11 months ago

It looks like there was a breaking change in ruff. Could try pip install --upgrade ruff which should repro locally. Then the failure says it an auto fixable change so ruff check --fix . from root should resolve. We probably should pin our ruff version.

jeremy-myers commented 11 months ago

My pre-commit failed to update ruff due to proxy issues and missed this error. I'm not sure how to resolve these extra args calls in cp_apr and fg_est.

ntjohnson1 commented 11 months ago

This is a general configuration issue. Unrelated to your change like you mentioned. It looks like pre-commit requires (or I just set it up this way), a pinned version of ruff here that hasn't conflicted with latest ruff until now. The simplest unblocking solution is probably to update our package ruff requirement here to match exactly. This will require rolling back your latest commit and downgrading ruff locally. That pinned version is why pre-commit passed last time but the pytest failed (I assumed it was just caching but clearing the cache wasn't successful).

Then can file a ticket to handle this cleaner. Ideally linking the two configs and not pinning to an EXACT ruff version.

ntjohnson1 commented 11 months ago

If that doesn't work I can debug it after business hours today on a separate branch.

jeremy-myers commented 11 months ago

@ntjohnson1 It sounds like I should wait for the pre-commit yaml and/or pyproject to be updated to unblock?

ntjohnson1 commented 11 months ago

Confirmed this unblocks and I think it is a reasonable temporary or even medium term solution since its only a dev requirement.

IMO I think the easiest approach is to cherry-pick the commit from there or just apply the change yourself and revert 75c0f1b6ffed2b7208cb7af8c1106d35468aa429. No need for a separate PR to unblock.

jeremy-myers commented 9 months ago

@ntjohnson1 It looks like this fix is now failing with black --check .. Is this another pinning issue?

ntjohnson1 commented 9 months ago

Boo. I assume so since it looks like we have it pinned here https://github.com/sandialabs/pyttb/blob/5751f02f631a78dcf0a06ec4f03dbda79eb5b98b/.pre-commit-config.yaml#L8. Should be quick to test if it fixes the issue.