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

pin HARK dependency in requirements.txt ? #152

Closed sbenthall closed 11 months ago

sbenthall commented 3 years ago

We release DemARKs on the same release cycle as HARK. (Currently, it has a 0.10.8 release)

We test DemARKs against HARK master so we can catch errors before the next release.

But we so far are not pinning the HARK dependency to the corresponding version of DemARK.

I think we should: version X of DemARK should have its HARK dependency pinned to X.

Right?

llorracc commented 3 years ago

Yes, I guess this is right; but it seems to be the reverse of the mysterious problem I just had. The DemARK has not been updated so it still has the "Now" in the variable names. But somehow myBinder is getting the version of HARK where the Now's have been removed. Since Mridul is the one who works at MyBinder, maybe he can help debug this!

On Tue, Feb 16, 2021 at 4:57 PM Sebastian Benthall notifications@github.com wrote:

We release DemARKs on the same release cycle as HARK. (Currently, it has a 0.10.8 release)

We test DemARKs against HARK master so we can catch errors before the next release.

But we so far are not pinning the HARK dependency to the corresponding version of DemARK.

I think we should: version X of DemARK should have its HARK dependency pinned to X.

Right?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/econ-ark/DemARK/issues/152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKCK77ELEMFJVA2VMKIVUTS7LS23ANCNFSM4XXGWQEQ .

--

sbenthall commented 3 years ago

Looks like the binder instance uses HARK master currently: https://github.com/econ-ark/DemARK/blob/master/binder/requirements.txt#L12

If it were pinned to 0.10.8, then it wouldn't have this issue.

llorracc commented 3 years ago

Oh, I assumed that Binder always used the last release.

Mridul, do you know if this varies from one notebook to another, or does it always use the latest HARK master?

Definitely should not use latest master, since we are allowing DemARKs and other content temporarily to break wrt latest master.

On Tue, Feb 16, 2021 at 5:33 PM Sebastian Benthall notifications@github.com wrote:

Looks like the binder instance uses HARK master currently: https://github.com/econ-ark/DemARK/blob/master/binder/requirements.txt#L12

If it were pinned to 0.10.8, then it wouldn't have this issue.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/econ-ark/DemARK/issues/152#issuecomment-780160740, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKCK773PPHAZCWA6IW42YDS7LXDVANCNFSM4XXGWQEQ .

--

MridulS commented 3 years ago

When we click on the binder launch button it takes us to https://mybinder.org/v2/gh/econ-ark/DemARK/master If we want to launch the DemARKs in 0.10.8 we should rather go to https://mybinder.org/v2/gh/econ-ark/DemARK/0.10.8. The 0.10.8 tag will also fail currently as I missed the step of swapping in explicit version of HARK in requirements.txt when making a release of DemARKs. This should become a part of the release procedure.

MridulS commented 3 years ago

I am not 100% sure how to document the https://mybinder.org/v2/gh/econ-ark/DemARK/master, https://mybinder.org/v2/gh/econ-ark/DemARK/0.10.8, https://mybinder.org/v2/gh/econ-ark/DemARK/0.10.7, ...... part. Should we just add this in the README?