Skip to content

Importing astropy.units fails with python -OO flag #10361

New issue
New issue
Closed
#10379
Closed
Importing astropy.units fails with python -OO flag#10361
#10379
Labels
Bugunits

Description

@anuppari
anuppari
opened on May 17, 2020

Description

Running a script that imports astropy.units with the python -OO optimization flag throws an exception.

Expected behavior

The script should not throw an exception.

Actual behavior

The exact output for the python script below is:

WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:192: No documentation string specified in function 'p_main' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:213: No documentation string specified in function 'p_division_product_of_units' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:224: No documentation string specified in function 'p_inverse_unit' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:230: No documentation string specified in function 'p_factor' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:238: No documentation string specified in function 'p_factor_float' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:253: No documentation string specified in function 'p_factor_int' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:275: No documentation string specified in function 'p_factor_fits' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:299: No documentation string specified in function 'p_product_of_units' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:312: No documentation string specified in function 'p_unit_expression' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:323: No documentation string specified in function 'p_unit_with_power' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:336: No documentation string specified in function 'p_numeric_power' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:346: No documentation string specified in function 'p_paren_expr' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:357: No documentation string specified in function 'p_frac' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:363: No documentation string specified in function 'p_sign' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:373: No documentation string specified in function 'p_product' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:380: No documentation string specified in function 'p_division' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:386: No documentation string specified in function 'p_power' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:393: No documentation string specified in function 'p_signed_int' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:399: No documentation string specified in function 'p_signed_float' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:406: No documentation string specified in function 'p_function_name' (ignored)
WARNING: ~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py:412: No documentation string specified in function 'p_function' (ignored)
Traceback (most recent call last):
  File "~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py", line 487, in _do_parse
    return cls._parse_unit(s, detailed_exception=False)
  File "~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py", line 465, in _parse_unit
    raise ValueError()
ValueError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "~/venv/lib/python3.6/site-packages/astropy/units/core.py", line 1819, in __call__
    return f.parse(s)
  File "~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py", line 472, in parse
    result = cls._do_parse(s, debug=debug)
  File "~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py", line 490, in _do_parse
    return cls._parser.parse(s, lexer=cls._lexer, debug=debug)
  File "~/venv/lib/python3.6/site-packages/astropy/utils/decorators.py", line 662, in __get__
    val = self.fget.__wrapped__(objtype)
  File "~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py", line 100, in _parser
    return cls._make_parser()
  File "~/venv/lib/python3.6/site-packages/astropy/units/format/generic.py", line 436, in _make_parser
    outputdir=os.path.dirname(__file__))
  File "~/venv/lib/python3.6/site-packages/astropy/extern/ply/yacc.py", line 3344, in yacc
    grammar.set_start(pinfo.start)
  File "~/venv/lib/python3.6/site-packages/astropy/extern/ply/yacc.py", line 1635, in set_start
    start = self.Productions[1].name
IndexError: list index out of range

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test_astropy.py", line 1, in <module>
    import astropy.units as apunit
  File "~/venv/lib/python3.6/site-packages/astropy/units/__init__.py", line 21, in <module>
    from . import astrophys
  File "~/venv/lib/python3.6/site-packages/astropy/units/astrophys.py", line 44, in <module>
    def_unit(['lyr', 'lightyear'], (_si.c * si.yr).to(si.m),
  File "~/venv/lib/python3.6/site-packages/astropy/constants/constant.py", line 42, in wrapper
    self.unit.to(inst.unit)
  File "~/venv/lib/python3.6/site-packages/astropy/units/quantity.py", line 741, in unit
    return self._unit
  File "~/venv/lib/python3.6/site-packages/astropy/utils/decorators.py", line 744, in __get__
    val = self.fget(obj)
  File "~/venv/lib/python3.6/site-packages/astropy/constants/constant.py", line 165, in _unit
    return Unit(self._unit_string)
  File "~/venv/lib/python3.6/site-packages/astropy/units/core.py", line 1833, in __call__
    raise ValueError(msg)
ValueError: 'm / (s)' did not parse as unit: list index out of range

Steps to Reproduce

  1. run python -OO test_script.py, where test_script.py is below:
import astropy.units as apunit

pass

System Details

Linux-4.15.0-99-generic-x86_64-with-Ubuntu-18.04-bionic
Python 3.6.9 (default, Apr 18 2020, 01:56:04)
[GCC 8.4.0]
Numpy 1.18.1
astropy 3.2.3
Scipy 1.4.1

Activity

  1. pllim commented on May 18, 2020

    @pllim
    pllim
    on May 18, 2020
    Member

    Thanks for reporting this, @anuppari !

    I can reproduce it with astropy 4.2.dev and Python 3.7.1 on Windows. Part of the traceback pointed to an external package called ply. If the problem lies in the external package, we can try to upgrade or submit a patch upstream, but nothing much we can do beyond that, I think. @mhvk ?

  2. pllim
    added
    units
    Bug
    on May 18, 2020

  3. mhvk commented on May 18, 2020

    @mhvk
    mhvk
    on May 18, 2020
    Contributor

    Reproduced on Debian linux with current(ish) master and python 3.8.2. Also with astropy 4.0 as packaged by Debian.
    Note that we bundle ply - I guess we could see if upgrading it to its latest version helps...

  4. mhvk commented on May 18, 2020

    @mhvk
    mhvk
    on May 18, 2020
    Contributor

    Actually, looking into this more, I think this may be impossible to fix: what -OO does is:

           -O     Remove  assert  statements and any code conditional on the value of __debug__; augment the filename for compiled (bytecode) files by adding
              .opt-1 before the .pyc extension.

       -OO    Do -O and also discard docstrings; change the filename for compiled (bytecode) files by adding .opt-2 before the .pyc extension.

    Since ply relies on docstrings for defining how to parse strings, this cannot possibly work.

    I guess what is still a question is whether one can pre-generate the parser to a sufficient degree that one can do without the docstrings..
    ...
    ...
    stackoverflow is one's friend:
    https://stackoverflow.com/questions/21457856/using-ply-with-python-oo
    suggests we can make some trivial changes that will help.
    Perhaps we may also need: http://www.dabeaz.com/ply/ply.html#ply_nn38b

  5. dhomeier commented on May 18, 2020

    @dhomeier
    dhomeier
    on May 18, 2020
    Contributor

    Thanks for reporting this, @anuppari !

    I can reproduce it with astropy 4.2.dev and Python 3.7.1 on Windows. Part of the traceback pointed to an external package called ply. If the problem lies in the external package, we can try to upgrade or submit a patch upstream, but nothing much we can do beyond that, I think. @mhvk ?

    Same with Python 3.7.7 and 3.8.3 on OS X; it is apparently a known issue upstream:
    https://github.com/dabeaz/ply/blob/master/README.md#requirements

    Might be more info for a possible fix on our side down that link:
    https://ply.readthedocs.io/en/latest/ply.html#using-python-oo-mode

  6. dhomeier commented on May 18, 2020

    @dhomeier
    dhomeier
    on May 18, 2020
    Contributor

    https://stackoverflow.com/questions/21457856/using-ply-with-python-oo
    suggests we can make some trivial changes that will help.
    Perhaps we may also need: http://www.dabeaz.com/ply/ply.html#ply_nn38b

    Seems it might be just that trivial: I've added optimize=True to the three parser = yacc.yacc() calls in cds.py, generic.py and ogip.py (the lex.lex() ones already had the flag), and that got me rid of the exception!

  7. dhomeier commented on May 18, 2020

    @dhomeier
    dhomeier
    on May 18, 2020
    Contributor

    I half-expected this would first require some initialisation of the code to “ first run Python in normal mode”, but it seems the standard install process already takes care of that, even when running setup.py -O2 on a fresh install.

  8. mhvk commented on May 18, 2020

    @mhvk
    mhvk
    on May 18, 2020
    Contributor

    @dhomeier - that's great! It sort-of makes sense that the standard install would deal with it, since it bundles the ply created files in generic_lextab.py and generic_parsetab.py. I guess one test would be to see whether regenerating them still works, as described in

    astropy/astropy/units/format/generic_parsetab.py

    Lines 4 to 10 in b8e8278

    # This file was automatically generated from ply. To re-generate this file,
    # remove it from this folder, then build astropy and run the tests in-place:
    #
    # python setup.py build_ext --inplace
    # pytest astropy/units
    #
    # You can then commit the changes to this file.

    Note that another table is created in coordinates (for Angle), so a PR might have to look at that too.

  9. dhomeier commented on May 18, 2020

    @dhomeier
    dhomeier
    on May 18, 2020
    Contributor
      # This file was automatically generated from ply. To re-generate this file, 
  # remove it from this folder, then build astropy and run the tests in-place: 
  # 
  #   python setup.py build_ext --inplace 
  #   pytest astropy/units`

    That works (the re-generated tables are identical except for some header comments).

    Note that another table is created in coordinates (for Angle), so a PR might have to look at that too.

    angle_utilities.py has a yacc.yacc call without optimize, too, but that does not seem to break
    python -OO -c 'import astropy.coordinates'.

  10. mhvk commented on May 18, 2020

    @mhvk
    mhvk
    on May 18, 2020
    Contributor

    Without the optimize, I think it might breaks actually doing something like

    python3 -OO
from astropy.coordinates import Angle
Angle("54°07'26.832\"")

    But I think this will all become clear with a quick PR....

    I was thinking of running the tests with -OO, but that would fail, I guess, since those are very much based on assert <thisorthat>, and those asserts would be optimized away...

  11. dhomeier commented on May 18, 2020

    @dhomeier
    dhomeier
    on May 18, 2020
    Contributor

    Yes, properly testing this seems the most complicated part about it.

  12. mhvk
    closed this as completedin #10379on May 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Bugunits

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions