search

phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

Improve public-facing documentation of mutation and population query parameters #259

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

Natural selection has 4 query parameters used to describe initial mutations and population on a per-screen basis. Relative to typical query parameters, they are quite complicated and tightly related. Those query parameters are:

introMutations introPopulation labMutations labPopulation

A PhET-iO client had this feedback about these query parameters:

Two things that trip me up slightly are having to differentiate between the intro and lab screen with the parameters and having to specify mutations in order to introduce mixed populations of bunnies. However, I understand the need for these things so maybe it's just a matter of making sure the user really gets that in the documentation? It wasn't clear to me that you must specify the mutations variable in order to use a mix of alleles in population. But, the examples helped me understand this in the end.

@amanda-phet @kathy-phet and I discussed in email (Subject: Natural Selection Update). Rather than reproduce the entire email thread here, here are the bits that summarize conclusions:

@amanda-phet said:

I can certainly make the language more friendly, but I've always thought it was really clear as written! It sounds like he figured it out and didn't really have trouble using the parameters, but I will work on how to reword the descriptions a bit. I don't think it would be much PhET time to do so.

@kathy-phet said:

Yes, I don't think we want to invest much in this -- just some work on explaining a bit more with a couple more examples.

@pixelzoom said:

... Since this feature is intended for use by phet and phet-io brands, the right place for that might not be in the Client Guide.

pixelzoom commented 3 years ago

@amanda-phet my understanding is that we're shooting for the week of March 29 to get this sim into dev testing. Then hopefully create the next RC branch immediately after that. Ideally, this issue would be addresssed before creating the RC branch. Otherwise we'll have to patch the RC branch.

pixelzoom commented 3 years ago

@amanda-phet is there any interest in doing this for the 1.3 release, which is currently in RC testing? If so, this work needs to be done asap, and patched into the 1.3 branch.

amanda-phet commented 3 years ago

Yes, I'd like to update these. Is today too late for that?

pixelzoom commented 3 years ago

No, today is not too late. I anticipate publishing the next RC early next week. But that depends on whether all of the issues are addressed.

In Slack, @amanda-phet asked:

If I worked on this today, would I edit phet-io-guide.md and then commit to master?

Chris Malley 11:13 AM That’s up to you and Kathy. My input (in the first comment of the issue) is:

... Since this feature is intended for use by phet and phet-io brands, the right place for that might not be in the Client Guide.

amanda-phet commented 3 years ago

Made a slight change that I think makes the population parameters a little clearer.

pixelzoom commented 3 years ago

@amanda-phet Reviewing https://github.com/phetsims/phet-io-client-guides/commit/0905067b2009e9497645250cf4537ef19d28edb1 …. I see that you changed “query-parameter value” (hyphenated) to “query parameter value” (unhyphenated) in one place, but not in the other. So now we're a bit inconsistent. I’m also not convinced this is a good change - “query-parameter value” (hyphenated) is grammatically correct, it’s functioning as an attributive compound adjective. https://www.editorgroup.com/blog/to-hyphenate-or-not-to-hyphenate/

amanda-phet commented 3 years ago

I only noticed it in the one place, but I see now there is one other use of "query-parameter value." I'm happy to change that back.

pixelzoom commented 3 years ago

In the above commits, I created the new branch for phet-io-clients-guides ("natural-selection-1.3"), cherry-picked the changes from master, fixed up dependencies, etc. -- all of the steps necessary for changing a dependency repo. This was relatively expensive, considering it only changed "The value" to "This query parameter" in 2 places. But that's the downside of having client guides separate from the sim repo.

I did a build of the 1.3 branch (grunt --brands=phet), and verified that the changes appear in the "PhET-iO Guide" that is accessed from the Wrapper Index.

@amanda-phet Given the small number of changes, do you want to spend QA resources verifying this in the next RC? Or would you like to just close this issue?

amanda-phet commented 3 years ago

This was relatively expensive, considering it only changed "The value" to "This query parameter" in 2 places.

When I checked the commit I noticed it only mentions those changes, but I actually added a sentence in two places as well. I'm not sure why the commit didn't show those additions.

@amanda-phet Given the small number of changes, do you want to spend QA resources verifying this in the next RC? Or would you like to just close this issue?

I am happy to just close this issue.

pixelzoom commented 3 years ago

@amanda-phet said:

When I checked the commit I noticed it only mentions those changes, but I actually added a sentence in two places as well. I'm not sure why the commit didn't show those additions.

Slack:

Chris Malley 11:15 AM About https://github.com/phetsims/natural-selection/issues/259#issuecomment-825788184 … Did you want to add the missing sentences? Or discuss?

Amanda McGarry 11:15 AM The sentences are there! They just aren’t highlighted as additions. I don’t know why.

Chris Malley 11:16 AM Oh, weird. OK, all good then. I’ll note in the issue.

pixelzoom commented 3 years ago

Reopening. The additional sentences added are incorrect.

Slack:

Amanda McGarry 11:17 AM It was just this explicit sentence: introMutations must be specified in order to set introPopulation.

Chris Malley 11:19 AM Uh oh… That’s not correct. introPopulation=2000 is valid without introMutations.

Amanda McGarry 11:20 AM That’s true…

pixelzoom commented 3 years ago

Discussed with @amanda-phet on Zoom. She's going to:

When done, please assign back to me for review and branch patching.

pixelzoom commented 3 years ago

@amanda-phet FYI... After reviewing, I made 2 changes in the above commit:

I'll proceed with patching the 3 commits above into the "natural-selection-1.3" branch of phet-io-client-guides.

pixelzoom commented 3 years ago

After cherry-picking the above commits to the branch, I discovered a formatting problem. See screenshot below - the double quotes around "Values" are messed up. I don't know why this is happening, and I don't think it's worth spending time to investigate. So I'm going to to change "Values" to Values.

screenshot_958
pixelzoom commented 3 years ago

Fixed in the above commit, see below. Still needed to patch the branch.

screenshot_959
pixelzoom commented 3 years ago

Branch was patched in the above commits.

This is ready for QA. I don't know how we're going to instruct QA to verify this. So I think it would be best if @amanda-phet verifies this in the next RC. One platform should be sufficient.

pixelzoom commented 3 years ago

@amanda-phet This issue is ready for you to verify in 1.3.0-rc.2. If everything looks OK, please check this issue off in https://github.com/phetsims/QA/issues/643, then close this issue.

High priority please, since the publication milestone is 4/30/21.

amanda-phet commented 3 years ago

Looks good to me!