search

econ-ark / REMARK

Replications and Explorations Made using the ARK
https://econ-ark.github.io/REMARK/
Apache License 2.0
20 stars 56 forks source link

BufferStock REMARK do_all.py crashes loading IPython shell #19

Closed sbenthall closed 4 years ago

sbenthall commented 5 years ago

Running this do_all.py from the command line: https://github.com/llorracc/BufferStockTheory/blob/5ccb22b72cb9fd46245b8954459ce5f44ab3c7ea/Code/Python/do_all.py

gets me the following error in the terminal:

Traceback (most recent call last):
  File "do_all.py", line 3, in <module>
    import BufferStockTheory
  File "/home/sb/projects/econ-ark/REMARK/REMARKs/BufferStockTheory/Code/Python/BufferStockTheory.py", line 154, in <module>
    get_ipython().run_line_magic('matplotlib', 'inline')
AttributeError: 'NoneType' object has no attribute 'run_line_magic'

This looks like it's because the get_ipython() method returns None when no InteractiveShell instance is registered: https://ipython.readthedocs.io/en/stable/api/generated/IPython.core.getipython.html

(Please let me know if these issues are better tracked to the BufferStockTheory repo in llorracc.)

MridulS commented 5 years ago

@sbenthall Are you running python do_all.py or ipython do_all.py? These scripts are supposed to work with ipython not the base python one.

llorracc commented 5 years ago

Several times I’ve had people say they can’t get the command line versions running and it’s because they are trying python rather than ipython from the command line.

We should add something like:

Pseudocode:

if [environment is python not ipython]: see whether ipython is available if so import it and print a mild warning message that the notebook should be run from the command line via ipython not python if ipython can't be imported, error out with a message saying something like 'this notebook must be run using ipython; please make ipython available on the command line and retry'

On Thu, Oct 31, 2019 at 4:29 AM Mridul Seth notifications@github.com wrote:

@sbenthall https://github.com/sbenthall Are you running python do_all.py or ipython do_all.py? These scripts are supposed to work with ipython not the base python one.

— 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/REMARK/issues/19?email_source=notifications&email_token=AAKCK7ZKRKJKDNOKEXDGWD3QRKJPRA5CNFSM4JF3Z4MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECW5O2I#issuecomment-548263785, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKCK74TEEJEIS63T6UH5FLQRKJPRANCNFSM4JF3Z4MA .

--

sbenthall commented 5 years ago

Ah, you're right. I probably was running it with python, to ipython.

I see now that in the comments at the start of the file, it says to use ipython.

This is maybe not the first place one would look. Maybe this note about using ipython could go in the README section about the do_all.py scripts.

https://github.com/econ-ark/REMARK#do_py

I think if it were documented, I wouldn't have made the mistake.

I think I would find the message proposed by @llorracc confusing because I didn't think the do_[].py scripts were notebooks. Is this do_[].py construct specific to REMARK as a coding pattern?

llorracc commented 5 years ago

The do.py files just run the auto generated BufferStockTheory.py file, so if the .ipynb master tests for ipython then that will flow through to what gets run by do_all.py.

On Thu, Oct 31, 2019 at 15:57 Sebastian Benthall notifications@github.com wrote:

Ah, you're right. I probably was running it with python, to ipython.

I see now that in the comments at the start of the file, it says to use ipython.

This is maybe not the first place one would look. Maybe this note about using ipython could go in the README section about the do_all.py scripts.

https://github.com/econ-ark/REMARK#do_py

I think if it were documented, I wouldn't have made the mistake.

I think I would find the message proposed by @llorracc https://github.com/llorracc confusing because I didn't think the do[].py scripts were notebooks. Is this do[].py construct specific to REMARK as a coding pattern?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/econ-ark/REMARK/issues/19?email_source=notifications&email_token=AAKCK73K35F7A6H27L2DG6LQRM2DBA5CNFSM4JF3Z4MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECZBZWI#issuecomment-548543705, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKCK76SNWS656WA6MYIUMLQRM2DBANCNFSM4JF3Z4MA .

-- Sent from Gmail Mobile

sbenthall commented 5 years ago

Ok. I see now that there are a few interwoven issues here.

I wonder if it would be possible to be more thoughtful about how these do_[].py scripts are designed, how that design is enforced when a new REMARK is included in this repository as a submodule, and whether any of the code necessary for complying with those design requirements could be in one place in the REMARK repository, rather than duplicated across every REMARK submodule.

llorracc commented 5 years ago

The solution for this is probably to add something in the first line of the Jupyter notebook that tests whether it is being run using ipython. When the [notebook].py jupytext file is autogenerated, if someone runs it using python [notebook].py they should get an error message.

sbenthall commented 4 years ago

This code will suffice for warning the user if they run a file in python instead of ipython:

try:
    get_ipython()
except:
    print("This file should be run with the `ipython` command, not `python'.")
llorracc commented 4 years ago

I just started to implement this in the BufferStockTheory REMARK, but noticed that the setup cell already contains:

from IPython import get_ipython # In case it was run from python instead of ipython

which should already be sufficient. (It will presumably halt with an error if ipython is not present).

So, could you go back and look again to see if you can figure out what was causing your earlier problems? It sounds like you've fixed it now, but it couldn't have been only the fact of running from the command line...

sbenthall commented 4 years ago

Ok, I'm looking into this again.

Currently, there are two do_all.py files. One is at the top level directory BufferStockTheory/ of that REMARK. The other is in BufferStockTheory/Code/Python.

Both of them now give me this error (I get a similar error whether running in IPython or Python):

$ ipython do_all.py 
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
~/projects/econ-ark/REMARK/REMARKs/BufferStockTheory/Code/Python/do_all.py in <module>
      1 # do_all.py is required for a REMARK; in this case, it just runs BufferStockTheory.py
      2 
----> 3 import BufferStockTheory
      4 
      5 

ModuleNotFoundError: No module named 'BufferStockTheory'

I believe this is because:

I'd recommend either:

  1. Replacing the .ipynb file with a .py file in the repository, or
  2. Adding a line to the do_all.py that runs the jupytext command to create the .py file
  3. Consolidate down to just one do_all.py file

My preferences is for (1) over (2). I think (3) would be good in any case.

llorracc commented 4 years ago

Closing because issues above are (mostly) resolved.