Adding end time and run duration to Compare Runs view #3378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Arpan Bhattacharya <tell.arpan.bhattacharya@gmail.com>
RealArpanBhattacharya
changed the title
github-actions
bot
added
the
area/uiux
|
@RealArpanBhattacharya Thanks for the PR and sorry for the very late reply :) Pushed a few commits to clean up the code, thanks for the contribution! |
Signed-off-by: harupy <hkawamura0130@gmail.com>
Signed-off-by: harupy <hkawamura0130@gmail.com>
|
I need to fix the row titles. |
Signed-off-by: harupy <hkawamura0130@gmail.com>
Signed-off-by: harupy <hkawamura0130@gmail.com>
|
Fixed
|
Comment on lines
276
to
278
| startTime: startTime ? Utils.formatTimestamp(startTime) : '(unknown)', | ||
| endTime: endTime ? Utils.formatTimestamp(endTime) : '(unknown)', | ||
| duration: startTime && endTime ? Utils.getDuration(startTime, endTime) : '(unknown)', |
There was a problem hiding this comment.
Can we translate the (unknown) string as well using formatMessage?
Comment on lines
297
to
330
| <> | ||
| <tr> | ||
| <th scope='row' className='head-value sticky-header' style={colWidthStyle}> | ||
| <FormattedMessage | ||
| defaultMessage='Start Time:' | ||
| // eslint-disable-next-line max-len | ||
| description='Row title for the start time of runs on the experiment compare runs page' | ||
| /> | ||
| </th> | ||
| {runTimeAttributes.map(({ runUuid, startTime }) => [runUuid, startTime]).map(renderCell)} | ||
| </tr> | ||
| <tr> | ||
| <th scope='row' className='head-value sticky-header' style={colWidthStyle}> | ||
| <FormattedMessage | ||
| defaultMessage='End Time:' | ||
| // eslint-disable-next-line max-len | ||
| description='Row title for the end time of runs on the experiment compare runs page' | ||
| /> | ||
| </th> | ||
| {runTimeAttributes.map(({ runUuid, endTime }) => [runUuid, endTime]).map(renderCell)} | ||
| </tr> | ||
| <tr> | ||
| <th scope='row' className='head-value sticky-header' style={colWidthStyle}> | ||
| <FormattedMessage | ||
| defaultMessage='Duration:' | ||
| // eslint-disable-next-line max-len | ||
| description='Row title for the duration of runs on the experiment compare runs page' | ||
| /> | ||
| </th> | ||
| {runTimeAttributes.map(({ runUuid, duration }) => [runUuid, duration]).map(renderCell)} | ||
| </tr> | ||
| </> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Can we dry the duplicate code out?
We can create an array with all the configs and loop over the array to render each table row. What do you think?
example:
const tableCells = [
{
translation = <FormattedMessage
defaultMessage='Start Time:'
// eslint-disable-next-line max-len
description='Row title for the start time of runs on the experiment compare runs page'
/>
},
{
translation = <FormattedMessage
defaultMessage='End Time:'
// eslint-disable-next-line max-len
description='Row title for the end time of runs on the experiment compare runs page'
/>
}
....
]
usage:
tableCells.map((cell) => {
<tr>
<th scope='row' className='head-value sticky-header' style={colWidthStyle}>
{cell.translation}
</th>
{runTimeAttributes.map(({ runUuid, startTime }) => [runUuid, startTime]).map(renderCell)}
</tr>
})
Signed-off-by: harupy <hkawamura0130@gmail.com>
Signed-off-by: harupy <hkawamura0130@gmail.com>
sunishsheth2009
approved these changes
Apr 15, 2022
Comment on lines
+210
to
+213
| "7kUi8J": { | ||
| "defaultMessage": "Duration:", | ||
| "description": "Row title for the duration of runs on the experiment compare runs page" | ||
| }, |
There was a problem hiding this comment.
Might have to run the i18n extract script again to add unknown to the default strings here
Signed-off-by: harupy <hkawamura0130@gmail.com>
Signed-off-by: harupy <hkawamura0130@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Signed-off-by: Arpan Bhattacharya tell.arpan.bhattacharya@gmail.com
What changes are proposed in this pull request?
In the "Compare Runs" section - display end time and run duration.
Addresses this issue.
How is this patch tested?
Still trying to figure out the automated test suite.
Screenshot:
Release Notes
Is this a user-facing change?
It is. In the "Compare Runs" section of the tracking UI, the end timestamp, and the duration of the experiment will be shown upon pushing this patch.
What component(s), interfaces, languages, and integrations does this PR affect?
Interface
area/uiux: Front-end, user experience, JavaScript, plottingHow should the PR be classified in the release notes? Choose one:
rn/none- No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section