Skip to content

Changes output['params'] from tuple to dict, enhancing output express… #238

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

Conversation

jeremy-myers
Copy link
Collaborator

@jeremy-myers jeremy-myers commented Sep 15, 2023

…iveness and ability to pass output['params'] to cp_als() by unpacking operator **.


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

…iveness and ability to pass output['params'] to cp_als() by unpacking operator **.
Copy link
Collaborator

@ntjohnson1 ntjohnson1 left a comment

Choose a reason for hiding this comment

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

Thanks. A similar few small requests and this also looks good to be merged by @dmdunla

@dmdunla
Copy link
Collaborator

dmdunla commented Sep 16, 2023

I do not understand the workflow here. This is merging a forked branch into a pyttb branch, but there is no pyttb PR to merge that branch:

@dmdunla
Copy link
Collaborator

dmdunla commented Sep 16, 2023

@jeremy-myers and @ntjohnson1 WARNING: I'm going to change the target branch for this PR from 236 to main, but it says that commits may be dropped.

@jeremy-myers The target branch for PRs should be main, even when coming from a branch from your fork of pyttb. I will update the dev docs to help clarify this for future development.

@dmdunla
Copy link
Collaborator

dmdunla commented Sep 16, 2023

@jeremy-myers, @ntjohnson1 FYI, here's the warning I see: Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.

@dmdunla dmdunla changed the base branch from 236-cp_als-does-not-accept-params-tuple-as-input to main September 16, 2023 15:40
@ntjohnson1
Copy link
Collaborator

I think it's a may rather than will. I think all these branches are relatively close to main so should be minimal risk. Will take a quick look. This one looks fine still.

@jeremy-myers jeremy-myers linked an issue Sep 16, 2023 that may be closed by this pull request
@dmdunla
Copy link
Collaborator

dmdunla commented Sep 16, 2023

@ntjohnson1 Except for the conflict? 😅

@ntjohnson1
Copy link
Collaborator

@ntjohnson1 Except for the conflict? 😅

... oops. It should be fixed now. Hopefully that didn't spam too many emails.

@dmdunla dmdunla merged commit 7d2e5a9 into sandialabs:main Sep 16, 2023
@jeremy-myers jeremy-myers deleted the 236-cp_als-does-not-accept-params-tuple-as-input branch September 28, 2023 18:27
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.

cp_als does not accept params tuple as input
3 participants