Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add multi-output support #396
feat: add multi-output support #396
Changes from 1 commit
7a164ed
71cf9b7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there was not a reason for us to have
--no-deps
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, but it breaks if we don't have dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird but ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not so weird imo, my understanding is that before our dependencies were a superset of
concrete-python
's. Now that they needz3
for bit-width assignment this breaks on our side because we wouldn't be installing it with--no-deps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok interesting then !
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it right to comment this ? why not keeping the
2.5.0-rc1
version to make poetry consider more realistic constraint in the dependencies to install ? this is an open question, not sure of this answerThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's best to comment if the value can't be installed by the user.
This way if a user tries to install main it will crash at import saying that it's missing Concrete Python instead of having the issue later on.
But not a strong opinion tbh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm got it yes, did not realize that for some times our public repo won't be possible to build from source by any external users since nightlies are private !
wonder if we could do something about that (tag the latest "CML commit usable from external users" ? avoid nightlies as much as possible ? don't know ! but not very important for now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how well pypi handles nightly but maybe we could publish the nightlies that we use to pypi if there is no IP blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's the main issue with nightly, I believe we don't want them to be public, only rc should
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should them for a rc once we see that the nightly is working for us?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it as well indeed, but at the same time @bcm-at-zama said that me way want to avoid too many public releases ! So maybe something to discuss about it, but from it looks like it may not be worth to put too much effort in all of this (we usually don't keep nightlies long enough). @andrei-stoian-zama seemed to say that we would ask for anticipated rc mostly if users ask for it