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

FieldIO instrumentation simplifications #330

Open
3 tasks done
pixelzoom opened this issue May 13, 2024 · 1 comment
Open
3 tasks done

FieldIO instrumentation simplifications #330

pixelzoom opened this issue May 13, 2024 · 1 comment
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented May 13, 2024

PDL was used as an exemplar for instrumenting Gas Properties. While working on phetsims/gas-properties#77 and phetsims/gas-properties#231 with @zepumph, we noted the following things about the implementation of FieldIO:

  • toStateObject is unnecessary. The default implementation that is derived from stateSchema does the same thing.
  • defaultDeserializationMethod is unnecessary. (Ask @zepumph why.)
  • applyState would be unnecessary if stateSchema used ReferenceArrayIO instead of ArrayIO. See ReferenceArrayIO's default applyState (line 37).

According to @zepumph, the above changes will change the PhET-iO API, but will not require migration processors.

@samreid
Copy link
Member

samreid commented May 17, 2024

I applied the changes above and tested in the state wrapper (I tested a VSM screen and the sampling screen). Seems to be working OK. @matthew-blackman can you please review/test?

@samreid samreid removed their assignment May 17, 2024
@matthew-blackman matthew-blackman added the dev:help-wanted Extra attention is needed label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants