Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinity inputs #2103

Merged
merged 20 commits into from
Oct 17, 2024
Merged

Conversation

anth-volk
Copy link
Collaborator

@anth-volk anth-volk commented Oct 17, 2024

Description

Fixes #2095

Changes

PolicyEngine/policyengine-api#1887 must be merged before this.

This PR makes a few relevant changes to enable infinity and -infinity inputs:

  • Adds JSON replacer and reviver functions to JSON.parse() and JSON.stringify(), then builds wrappers on top of these and uses them wherever we anticipate possible receiving an infinite value (anything policy-related)
  • Adds a wrapper on top of this for Response.json(), which has no means of specifying a custom reviver, and replaces all uses of Response.json() with the new wrapper
  • Replaces StableInputNumber in the ParameterEditor component with Ant Design's InputNumber, then makes the input a controlled input to allow custom updating. This also required some changes to the underlying display logic.
  • Adds a small custom input toolbar present for all inputs except Booleans, percentages, and Enums. This bar has a button to open, and when opened, displays an infinity and negative infinity symbol.

There remain four unaddressed issues that I've chosen to leave unaddressed to make sure we can push this as quickly as possible. If pushed, I can address these at a different time:

  1. The parameter history chart does not display infinite values. This is due to limitations within Plotly, and I've yet to find a good solution.
  2. No tests have been added, but I intend to do so down the road, considering the priority level.
  3. The "Reproduce in Python" page does not properly set relevant reform values to infinity. This shouldn't be too significant.
  4. There's a visual bug in the number input box when a parameter has a default value of Infinity; for some reason, instead of displaying the infinity symbol, it will display "Infinity," and there are some quirks around that, as well.

I have also tested manually that this does generate a response from the back end when running locally with the required API changes. That said, I had no value to test against, so am unclear on the accuracy of the output.

Screenshots

AwesomeScreenshot-10_17_2024.12_57_25AM.mp4

Tests

None have been added.

Copy link
Contributor

@nikhilwoodruff nikhilwoodruff left a comment

Choose a reason for hiding this comment

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

Good to merge after adding shading to the open/collapse button, and passing tests.

@anth-volk anth-volk merged commit 30910d9 into PolicyEngine:master Oct 17, 2024
2 checks passed
@anth-volk anth-volk deleted the feat/2095-infinity-input branch October 17, 2024 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add the ability to set Infinity and -Infinity as input values
2 participants