Skip to content
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

Optional inputs #3842

Draft
wants to merge 31 commits into
base: main
Choose a base branch
from
Draft

Optional inputs #3842

wants to merge 31 commits into from

Conversation

amickan
Copy link
Contributor

@amickan amickan commented Feb 20, 2025

Final PR for https://github.com/DIAGNijmegen/rse-roadmap/issues/153

I will merge with main once again after my vacation, but leaving this here already in draft mode.

amickan and others added 30 commits December 6, 2024 10:44
First PR for DIAGNijmegen/rse-roadmap#153

This adds the `AlgorithmInterface` model, as well as the necessary
through table, and removes the inputs and outputs fields from the
algorithm form.

The following restrictions apply:
- AlgorithmInterfaces cannot be deleted (protection on model level and
admin form level)
- AlgorithmInterfaces cannot be updated (protection on admin form level)
- AlgorithmInterfaces are unique (i.e. input and output combinations are
unique, we check for that in the form)
- An algorithm can only have 1 default algorithm interface
- There can only be one entry per algorithm and interface in the through
table.

---------

Co-authored-by: James Meakin <[email protected]>
Part of DIAGNijmegen/rse-roadmap#153

This adds a create and list view for algorithm interfaces that is
accessible to algorithm editors with the global `add_algorithm`
permission.

Create view: 

![image](https://github.com/user-attachments/assets/4af87e6b-ef54-4a79-91e5-6df9f4ffb8c3)

List view: 

![image](https://github.com/user-attachments/assets/221b9cc4-bf01-43e3-833c-db6a082106cb)

---------

Co-authored-by: James Meakin <[email protected]>
Adds a check to both the form and the model that ensures that an
algorithm has a default interface. We're not using the through model
directly to create an interface for an algorithm through the UI, hence
the check in 2 places.
This enables selecting an interface when trying out an algorithm.
Interface selection happens in a separate form and view, but is
unnecessary if an algorithm has only 1 interface.


![image](https://github.com/user-attachments/assets/ee3c3f99-1b71-4e26-9db3-223d8625d5f6)


Part of DIAGNijmegen/rse-roadmap#153
This enables users with the `add_algorithm` permission to remove
interfaces from their algorithm.

Part of DIAGNijmegen/rse-roadmap#153
Adds restriction that interfaces for an algorithm need to have unique
sets of inputs, discussed
[here](DIAGNijmegen/rse-roadmap#153 (comment)).
Also fixes an oversights from an earlier PR.
This adds input-to-interface matching to the JobPostSerializer so that
algorithm jobs created through the API also have the interface
configured.
This also adds a serializer for algorithm interfaces.

Part of DIAGNijmegen/rse-roadmap#153
I added an input/output count annotation to the
`AlgorithmInterfaceManager` in my last PR, but had not tested the
migrations again. In my migration check for the phase interfaces, I
realized that the migrations break with that addition since they don't
have access to custom manager methods. So I'm moving the annotation to a
separate function here. The upside of this is that I can now use it in 2
other places where we're making the same annotation. So I'm calling this
a win.

Note that this does not add or change functionality, it just moves the
code. So I didn't spend time on adding a test for the new function - it
is indirectly covered by tests of all the places where it is used.
Discussed with James that we don't need the option to mark an interface
as default, so removing it here.
This adds a `algorithm_interfaces` M2M to the `Phase` model and adds a
custom through model for it.
It also removes the input / output configuration option from both the
phase admin and the `ConfigureAlgorithmPhases` view.
In anticipation of PR 2, it refactors the interface create form so that
it can be easily used for both phases and algorithms alike.

Views to create and manage interfaces for phases will be added in a
separate PR.

Part of DIAGNijmegen/rse-roadmap#153
This adds the management views for algorithm interfaces for phases. 
The views are only accessible to staff users, and only for algorithm
submission phases that are not external.
The logic behind the views is the same as for the algorithm interfaces,
but it only made sense to refactor the create view logic, and parts of
the templates.


![image](https://github.com/user-attachments/assets/b9c3d86b-fd2e-4ee4-a558-7bef40f15ab5)


Part of DIAGNijmegen/rse-roadmap#153
This unifies the display of algorithm interface information in various
locations.

Part of DIAGNijmegen/rse-roadmap#153
This renames `interface` to `socket` on user-facing views and forms. 
I did not change the urls that include the word `interface`. That seems
less important to me for now. We will need to add permanent redirects
when we do change them.

I based this on the feature branch for optional inputs since a lot of
the places that require changes are touched in this branch. The users
can wait 1 week for the renaming I think.

The only places where the user is still confronted with the word
`interface` now (as far as I can tell) is the API. Updating that is more
complex.
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.

1 participant