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

Marginal 2D diagnostic tool #1807

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Marginal 2D diagnostic tool #1807

wants to merge 2 commits into from

Conversation

ndmlny-qs
Copy link
Contributor

Motivation

Continued work on the diagnostics tools.

Changes proposed

  • Removed items in the pointStatistic.ts module and placed them in other modules for better structure. Updated all modules that needed to be updated due to the move.
  • Moved the method updateAxisLabel to a new module called utils/plottingUtils.ts for reuse.
  • Significantly reduced the complexity in webpack.config.js and tsconfig.json.
  • Updated the viz.py file to include the new marginal 2d tool.
  • Added the marginal 2d tool.

Test Plan

Just visual inspection for now, test to come soon.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • The title of my pull request is a short description of the requested changes.

- Removed items in the `pointStatistic.ts` module and placed them in
  other modules for better structure. Updated all modules that needed to
  be updated due to the move.
- Moved the method `updateAxisLabel` to a new module called
  `utils/plottingUtils.ts` for reuse.
- Significantly reduced the complexity in `webpack.config.js` and
  `tsconfig.json`.
- Updated the `viz.py` file to include the new marginal 2d tool.
- Added the marginal 2d tool.
@ndmlny-qs ndmlny-qs added the enhancement New feature or request label Nov 2, 2022
@ndmlny-qs ndmlny-qs self-assigned this Nov 2, 2022
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 2, 2022
@facebook-github-bot
Copy link
Contributor

@horizon-blue has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@horizon-blue
Copy link
Contributor

Looks like there're some version updates in yarn.lock but there's no change in package.json. Are those updates necessary? I'm asking because due to security reason, we have to check in third-party codes into our internal codebase and load the packages from there instead of downloading them from Yarn/NPM each time we build the library. So every time we introduce a new dependency, we will have to import and check in a bunch of new packages.

Our internal jobs currently fails because it cannot find the node modules that are being used, so I just want to check with you to see if the versions that were in yarn.lock are sufficient for what we need? If so, then it will save me from the hassle of importing a bunch of new packages every time there's a change to our diagnostic module :)

@ndmlny-qs
Copy link
Contributor Author

This is totally my mistake. I was trying out Bokeh 3.0 and did an update with yarn, which is why the lock file changed. I completely forgot to recheckout the lock file and not commit the changes. I'll revert now those changes.

@facebook-github-bot
Copy link
Contributor

@ndmlny-qs has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@horizon-blue has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants