-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Related Jobs in Dataset and Model view #767
Conversation
elif isinstance(job, generic.GenericDatasetJob): | ||
return generic.views.show(job) | ||
return generic.views.show(job, related_jobs=related_jobs) |
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 you forgot to add this parameter to the show()
function in digits/dataset/generic/views.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.
Ah that's because the file digits/dataset/generic/views.py
is brand new. Didn't catch that, well spotted.
Excellent points, I reduced down the complexity to just one function. I hope this feature will increase the ease for work for us Digitizers. |
Wow. Nice! 🤘 |
related_jobs = [] | ||
|
||
if isinstance(job, ModelJob): | ||
datajob = job.dataset |
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.
Thanks for the updates! If you don't mind a couple more comments... there you can do:
related_jobs.append(datajob)
so you can take lines 233-235 out.
Keep going like this and we will run out of lines of code. Squashed yet again plz review @gheinrich . |
<div class="panel-group"> | ||
<!-- Related model jobs --> | ||
{% for r_job in related_jobs %} | ||
{% if prev_job_model != r_job.job_type() %} |
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.
it might have been more explicit to call this prev_job_type
but I don't think it's a big deal
Great PR, very useful! I tested it (also on some "generic" data jobs) and it works well. Thanks! |
Perfect, thanks for the review. |
Okay, updated the line. Want me to squash? |
Yep, looks good to me! |
Reduced complexity of implementation Newline fixes Removed a print Fixes Fixed link for future resilience Typo
Squashed. |
It's kinda weird that the current model shows up below as a "related" job, no? I can't see why I would ever click on that. |
Related Jobs in Dataset and Model view
See #667 , revised version of #668.
See below for screenshots. Very straightforward.
Ready for review @lukeyeager (#667 (comment))
Dataset View:
Model View: