Skip to content

Fitting result models and views#281

Merged
jonc125 merged 21 commits intomasterfrom
203-fitting-result
Oct 2, 2020
Merged

Fitting result models and views#281
jonc125 merged 21 commits intomasterfrom
203-fitting-result

Conversation

@helenst
Copy link
Copy Markdown
Contributor

@helenst helenst commented Sep 1, 2020

Adapted from #241 to address #134.

I've updated the comparison js to account for the new types on the FittingResult model. It should still work for Experiments.

@helenst helenst changed the title Fitting result models Fitting result models and views Sep 1, 2020
Copy link
Copy Markdown
Contributor

@jonc125 jonc125 left a comment

Choose a reason for hiding this comment

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

I've not gone through all of this yet, but the models and views certainly look to be along the right lines!

Comment thread weblab/fitting/views.py
model = FittingResult

def get_success_url(self, *args, **kwargs):
return reverse('experiments:list') + '?show_fits=true'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will need to change once we have a proper matrix view for fitting results!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or it could redirect to one of the simpler views from #276 in the interim?

Comment thread weblab/templates/fitting/fittingresultversion_detail.html Outdated
Comment thread weblab/templates/fitting/fittingresultversion_detail.html Outdated
Comment thread weblab/templates/fitting/fittingresultversion_detail.html Outdated
Comment thread weblab/templates/fitting/fittingresultversion_detail.html Outdated
Comment thread weblab/templates/fitting/fittingresultversion_detail.html Outdated
Comment thread weblab/experiments/processing.py
Comment thread weblab/fitting/processing.py Outdated
Copy link
Copy Markdown
Contributor

@jonc125 jonc125 left a comment

Choose a reason for hiding this comment

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

I've now gone through the rest of the PR; all looking good so far.

Currently, these are equivalent to the experiment comparison views. They
will need adapting to fitting experiments.
@helenst helenst linked an issue Sep 9, 2020 that may be closed by this pull request
13 tasks
Include dataset and fitting spec names / versions
Include fitting spec and dataset names and versions, different
possibilities for different entity / version comparisons
@helenst helenst marked this pull request as ready for review September 10, 2020 09:00
Copy link
Copy Markdown
Contributor

@jonc125 jonc125 left a comment

Choose a reason for hiding this comment

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

Fitting experiments are still running so I can't compare results yet, but I've found 2 minor issues from what I can see so far.

Comment thread weblab/templates/fitting/fittingresultversion_detail.html
Comment thread weblab/templates/fitting/fittingresultversion_detail.html
@jonc125
Copy link
Copy Markdown
Contributor

jonc125 commented Sep 25, 2020

I've now played around with the compare view, and deletion, and both work as expected. So apart from the issues noted above this is good to merge. Would you rather fix the issues here or roll them into #290 @helenst ?

@helenst
Copy link
Copy Markdown
Contributor Author

helenst commented Sep 25, 2020

Great! I think at this point it'd be easier to roll it all into one :)

@jonc125 jonc125 merged commit 0576347 into master Oct 2, 2020
@jonc125 jonc125 deleted the 203-fitting-result branch October 2, 2020 10:58
MauriceHendrix pushed a commit that referenced this pull request Jul 22, 2021
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.

Add FittingResult model

2 participants