Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

[webui] Update default metric tooltip #3316

Merged
merged 5 commits into from
Jan 29, 2021

Conversation

Lijiaoa
Copy link
Contributor

@Lijiaoa Lijiaoa commented Jan 18, 2021

@@ -25,6 +25,8 @@ const EmptyGraph = {
interface DefaultPointProps {
trialIds: string[];
visible: boolean;
chartHeight: number;
isHasbestCurve: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest hasBestCurve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@Lijiaoa Lijiaoa closed this Jan 20, 2021
@Lijiaoa Lijiaoa reopened this Jan 20, 2021
@@ -6,8 +6,13 @@ import { isManagerExperimentPage } from './static/function';
import NavCon from './components/NavCon';
import MessageInfo from './components/modals/MessageInfo';
import { SlideNavBtns } from './components/slideNav/SlideNavBtns';
const echarts = require('echarts/lib/echarts');
echarts.registerTheme('my_theme', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "nni_theme" or "nniTheme" is a better name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, will change it to nni_theme

<Accuracy accuracyData={accuracyGraphData} accNodata={noDataMessage} />
<DefaultPoint
trialIds={bestTrials.map(trial => trial.info.trialJobId)}
visible={true}
Copy link
Contributor

@liuzhe-lz liuzhe-lz Jan 22, 2021

Choose a reason for hiding this comment

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

I think "visible" can be handled in this level. No need to pass a property into DefaultPoint.
Correct me if it's not the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, here no need this visible. And also no need whichGraph. whichGraph is appeared in v0.8 for ant design. It should be deleted when using fabric/fluent because current tab will be uninstall when you choose other tab. So webui doesn't need to consider current tab when page refresh. Thanks for your comment.

@J-shang J-shang mentioned this pull request Jan 22, 2021
94 tasks
@Lijiaoa
Copy link
Contributor Author

Lijiaoa commented Jan 25, 2021

image

Test also find that: hyper-parameter | Duration | Intermediate result graph will render twice when page refreshing. And drawing Default metric graph function is in render() component. It doesn't correct. These status maybe will fix in next release.

It seems that is by design: related doc

@Lijiaoa Lijiaoa closed this Jan 25, 2021
@Lijiaoa Lijiaoa reopened this Jan 25, 2021
@Lijiaoa Lijiaoa closed this Jan 27, 2021
@Lijiaoa Lijiaoa reopened this Jan 27, 2021
@Lijiaoa Lijiaoa closed this Jan 27, 2021
@Lijiaoa Lijiaoa reopened this Jan 27, 2021
@Lijiaoa Lijiaoa closed this Jan 28, 2021
@Lijiaoa Lijiaoa reopened this Jan 28, 2021
@J-shang J-shang merged commit bd0eae1 into microsoft:master Jan 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants