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
Added a generic convert function strategy #163
Added a generic convert function strategy #163
Changes from all commits
9bcb26c
15d4b3e
dd82900
8cd02b6
68870b9
9176ef6
5e8aa9e
721da44
2c9c3ce
e768132
0021739
01f3bf1
521b87c
6e2d71e
66e7de5
3441483
fcab3fa
2a589ac
fc5f24c
e27e824
3ed520c
62939d7
58bfc73
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.
Is there an issue for addressing the 3.11 problems?
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.
Added issue #168
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.
why are 'requirements.*' preferred over setup.py?
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.
Copy of my answer to Francesca:
I splittet it into two lines, because I wanted to install the requirements with the update (-U) option, which doesn't make sense for development (-e .) installation.
But it might not be necessary. I tried to change a lot of things before the CI on GitHub finally went through. It could very well be the change in line 91-92 that did the work...
This file was deleted.
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.
Remember to update the config with the relevant fields from the session
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.
Yes, good point. That is needed if we want a later filter to have an effect.
I was thinking about encouraging not specifying labels in the configuration. But fetching the label from the session could be useful. However, a single
label
in the session will not do, since each input and output has an optional label, so we need to be more specific when specifying labels in the session.Would it make sense to allow variable substitutions in the configuration of a partial pipeline, like
where
${temimage}
and${precipitate_statistics}
are substituted from the session.On the pros side, this would allow templated partial pipelines with improved re-usability.
On the conc side, it will be an extra layer of complexity.
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 am not completely sold on the idea of labels. Also, if we start creating declarative partial pipelines with variables, they become hard to exchange/share between instances. Why not refer to a resource directly?
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.
You have a good point. I agree that a partial pipeline should document a single data source or sink and in general not contain variables. However, there are cases where fetching parameters from the configuration are useful. One case is a partial pipeline documenting a SQL database. The documentation of the database should be fix, while the query may vary each time we execute the pipeline. Another possible use case is to specify the label a parser should use when storing an newly created instance into the collection and correspondingly, the label a generator should use when fetching an instance from the collection. In this case the labels may be variables when documenting the partial pipelines, but must be assigned and internally consistent before executing the full pipeline.
While variables is an easy and flexible way to assign consistent labels across partial pipelines, it may also open a can of worms of potential misuse.
Furthermore, in the common case that we only have one instance of a given entity in the collection, we don't need labels, since we can refer to the instance by specifying the entity in the configuration.
So if we solve the issue of assigning the labels in strategies that may refer to multiple instances (like the convert strategy) without variables, it might be a good idea to avoid variables in the partial pipelines stored in the knowledge base.
However, for populate the knowledge base with partial pipelines of a set of similar data sources, I think that templates with variables would be very useful. In this case, all substitutions should be done before storing into the knowledge base. Such a template utility may in this case live outside oteapi.
This file was deleted.