PR for #237 + #238: PROOF + FACT + MATH

[santiagohermo]

This is a review for production tasks PROOF (#237) and FACT (#238).

Annotated PDFs related to PROOF:

• Replies to Jon Roth here
• Replies to @gabrieleborg here
• Comments after @santiagohermo's review here

Annotated PDF related to FACT:

• See here

To-dos in the review (with tentative reviewers):

• [x] Go over comments and replies to Jon Roth (@gabrieleborg)
• [x] Go over comments and replies to Gabri and Juan Pereira (@gabrieleborg)
• [ ] ~Change MW figures to go from January 2010 to June 2020 to match the span of the MW panel (@diegogentilepassaro)~
  • --> See #234
• [x] Go over my annotations in the PDF here and confirm everything looks good (@martingallardo23)
• [x] Go over changes related to FACT and annotated pdf (@martingallardo23)
• [x] Review the changes in the code (@martingallardo23)

[diegogentilepassaro]

@santiagohermo, al final esta tarea de arreglar esos MW plots en descriptive esta resultando bastante más complicada de lo que pensaba. Fijate que como hay ZIPcodes que cruzan las fronteras del estado hay veces que el statutory_mw que viene del state está con decimales y eso genera que algunos estén apenas por arriba de 7.25. Podés comprobarlo con br if inrange(state_mw, 7.26, 7.30) después de la linea 20 en descriptive/mw_us/code/plot_mw. Ahí podés ver que la varianble de statutory está con decimales. La pregunta es que hacer con esos. Si los dejamos el state plot estaría bien.

Otro problema: en el plot de los mw locales hay dos temas (i) hay una jurisdicción que en enero 2020 el mw cae y eso se ve en el plot @martingallardo23 podés chequear que ese cambio efectivamente es correcto? (ii) solo esa jurisdicción tiene data de mw local (county_mw o local_mw) más allá de diciembre de 2019 y la tiene solo por enero. @martingallardo23 puede ser que los archivos donde viven los cambios substate no vayan hasta junio de 2020 o que los recortemos en algún paso en base o derived?

[diegogentilepassaro]

Ya descubrí lo que pasa con los MW locales:

• En enero del 2020 muchos estados pasan un nuevo MW. Entonces el max state-fed o max state-county-fed cambia y muchos county y local MW dejan de ser binding por lo que algunos meses al final de los datos se droppean en las lineas 47 y 58 de descriptive/mw_us/code/plot_mw.do. Estoy arreglandolo a ver como se ve quedandose todas las jurosdicciones que alguna vez tuvieron un MW por arriba del max state-fed (en el caso de los county) o state-county-fed (en el caso de las ciudades).

[diegogentilepassaro]

@santiagohermo implemented a fix in 1d18925. What do you think?

[santiagohermo]

Gracias @diegogentilepassaro! Sobre este cambio me parece que el plot anterior se ve mejor porque da una sensación de cuando los substate pibes arrancan con el MW (de hecho, mencionamos este tema en el texto).

Sobre esto. Entiendo la existencia de esos ZIP codes molestos. Lo que decía acá es que hay una línea que es casi 7.25 que queda rara y quisiera tirar. Fue agregada acá:

Creo que para eso alcanzaría con redondear el state en 0.001, como antes de este commit.

---

Sobre el MW long run, que cambiaste en https://github.com/diegogentilepassaro/min_wage_rent/pull/242/commits/df016f28432545127516cf469548e8d2c134241b. Mirando el plot ahora los colores van de 0 a 60, y antes iban de 0 a 100. Cómo puede ser?

Tiene pinta de que hay algo mal con el MW panel, intuyo que en los sub-state MW changes. Lo podés revisar?

---

Propuesta de cómo seguir:

1. Revertí todos los commit cambiando el long run plot y el MW by jurisdiction plot
2. Cerramos este PR focalizando en el proofreading
3. Abrimos un nuevo issue para revisar (rápido) el MW panel y estos plots.

Qué te parece? Me parece mejor así no se arma quilombo en este PR, que tenía un objetivo muy distinto.

[santiagohermo]

Note: branch updated from master

[santiagohermo]

I went ahead and implemented the plan I proposed here @diegogentilepassaro. See #243

Updating MW figures is off this issue.

[martingallardo23]

Go over my annotations in the PDF https://github.com/diegogentilepassaro/min_wage_rent/issues/237#issuecomment-1188307634 and confirm everything looks good (@diegogentilepassaro)

I'll take this one @diegogentilepassaro

[martingallardo23]

Cuatro cosas mínimas sobre tu review @santiagohermo. Las marqué con comentarios en naranja acá

min_wage_rent_SH_MG.pdf

[santiagohermo]

Cuatro cosas mínimas sobre tu review @santiagohermo. Las marqué con comentarios en naranja acá

min_wage_rent_SH_MG.pdf

Gracias @martingallardo23! I marked as resolved this point the opening comment.

Replies in min_wage_rent_SH_MG_SH.pdf A few edits in https://github.com/diegogentilepassaro/min_wage_rent/pull/242/commits/1c202299b8cc63963de0d82a80481e0c784f1ff5

[gabrieleborg]

@santiagohermo @diegogentilepassaro I went over comments (both Santi's reply to my and JP's comments, and JR comments), but I didn't find anything to change! I agree with the edits you did @santiagohermo :)

Thanks again for leading this final push! l Please let me know when it's finally submitted!

[santiagohermo]

Thanks @gabrieleborg! I'm all set to finish this off then :)

[santiagohermo]

Note: branch updated from master after merging #243. I will re-run /analysis/ and wrap up this issue.

[santiagohermo]

Summaries here and here.