econ-ark / DemARK

Demonstrations of how to use material in the Econ-ARK
https://econ-ark.github.io/DemARK/
Apache License 2.0
33 stars 93 forks source link

Remove `FashionVictim` DEMARK or peg econ-ark dependency to 0.10.2 #77

Closed sbenthall closed 4 years ago

sbenthall commented 4 years ago

The FashionVictim DEMARK imports code from HARK:

https://github.com/econ-ark/DemARK/blob/master/notebooks/Fashion-Victim-Model.py#L52

After this, it appears to execute the exact same code that is in the Fashion-Victim-Model main() method: https://github.com/econ-ark/DemARK/blob/master/notebooks/Fashion-Victim-Model.py#L123 https://github.com/econ-ark/HARK/blob/master/HARK/FashionVictim/FashionVictimModel.py#L414

In other words, this DEMARK is just a duplicate of what's in HARK, with some copy-and-pasted code.

FashionVictim is slated to be taken out of the HARK source library and into a HARK examples/ directory. https://github.com/econ-ark/HARK/issues/440

It has been argued that since FashionVictim is really a kind of tutorial documentation, not material for economics students, it's better for it to be in examples/ than to be a DemARK.

Either:

If we decide on the latter for some reason, I hope we put some thought into how to design this better to avoid code duplication across repositories.

llorracc commented 4 years ago

Let's make FashionVictim an "example" notebook, and remove it from the DemARK.

llorracc commented 4 years ago

@sbenthall will integrate this with https://github.com/econ-ark/HARK/issues/446

sbenthall commented 4 years ago

Fixed with https://github.com/econ-ark/DemARK/commit/e4a30aabde9f318dc86fd87753f63032dc5bc862#diff-c2365e874ecb861a659e5215ff17c338