Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More robust evaluation of assignments #57

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

amyreese
Copy link
Member

@amyreese amyreese commented Feb 8, 2022

Improves assignment evaluation in a number of ways:

  • Evaluates assignments in reverse line-number order, from bottom to top
  • Tracks target name and line number when evaluating recursively
  • Only considers recursive assignments that occur before the current target line
  • Attempts evaluating earlier assignments if later assignments are inconclusive
  • Prevent evaluating binary operands if either side evaluates to "??"
  • Add support for "augmented assignments" like +=

This enables support for evaluating self-referential assignments like:

foo = foo + bar

or

foo += bar

Adds a basic test covering both self-referential assignment types.

Fixes #56

Tracks the `target_name` that we are recursively evaluating, and
short-circuits evaluation if we attempt to evaluate that name again.
Also allows the assignment evaluation to try multiple assignments until
either a real value is found, or all assignments are exhausted. Also
prevents combining lhs and rhs of binary addition if either side is the
`"??"` sentinel value.

Fixes python-packaging#56
@amyreese amyreese added the bug Something isn't working label Feb 8, 2022
@amyreese amyreese requested a review from thatch February 8, 2022 03:52
@amyreese amyreese self-assigned this Feb 8, 2022
@amyreese
Copy link
Member Author

amyreese commented Feb 8, 2022

Final metadata evaluated against onnx repo:

(.venv) jreese@mordin ~/workspace/dowsing onnx  » python -m dowsing.pep517 ~/scratch/onnxruntime | jq .get_metadata
{
  "metadata_version": "2.1",
  "name": "onnxruntime-openvino",
  "version": "??",
  "summary": "ONNX Runtime is a runtime accelerator for Machine Learning models",
  "description": "??",
  "keywords": "onnx machine learning",
  "home_page": "https://onnxruntime.ai",
  "author": "Microsoft Corporation",
  "author_email": "[email protected]",
  "license": "MIT License",
  "classifiers": [
    "Development Status :: 5 - Production/Stable",
    "Intended Audience :: Developers",
    "License :: OSI Approved :: MIT License",
    "Operating System :: POSIX :: Linux",
    "Topic :: Scientific/Engineering",
    "Topic :: Scientific/Engineering :: Mathematics",
    "Topic :: Scientific/Engineering :: Artificial Intelligence",
    "Topic :: Software Development",
    "Topic :: Software Development :: Libraries",
    "Topic :: Software Development :: Libraries :: Python Modules",
    "Programming Language :: Python",
    "Programming Language :: Python :: 3 :: Only",
    "Programming Language :: Python :: 3.6",
    "Programming Language :: Python :: 3.7",
    "Programming Language :: Python :: 3.8",
    "Programming Language :: Python :: 3.9"
  ],
  "requires_dist": "??",
  "package_data": "??",
  "packages": [
    "onnxruntime",
    "onnxruntime.backend",
    "onnxruntime.capi",
    "onnxruntime.capi.training",
    "onnxruntime.datasets",
    "onnxruntime.tools",
    "onnxruntime.tools.ort_format_model",
    "onnxruntime.tools.ort_format_model.ort_flatbuffers_py",
    "onnxruntime.tools.ort_format_model.ort_flatbuffers_py.fbs",
    "onnxruntime.quantization",
    "onnxruntime.quantization.operators",
    "onnxruntime.quantization.CalTableFlatBuffers",
    "onnxruntime.transformers",
    "onnxruntime.transformers.longformer"
  ],
  "packages_dict": {
    "onnxruntime": "onnxruntime",
    "onnxruntime.backend": "onnxruntime/backend",
    "onnxruntime.capi": "onnxruntime/capi",
    "onnxruntime.capi.training": "onnxruntime/capi/training",
    "onnxruntime.datasets": "onnxruntime/datasets",
    "onnxruntime.tools": "onnxruntime/tools",
    "onnxruntime.tools.ort_format_model": "onnxruntime/tools/ort_format_model",
    "onnxruntime.tools.ort_format_model.ort_flatbuffers_py": "onnxruntime/tools/ort_format_model/ort_flatbuffers_py",
    "onnxruntime.tools.ort_format_model.ort_flatbuffers_py.fbs": "onnxruntime/tools/ort_format_model/ort_flatbuffers_py/fbs",
    "onnxruntime.quantization": "onnxruntime/quantization",
    "onnxruntime.quantization.operators": "onnxruntime/quantization/operators",
    "onnxruntime.quantization.CalTableFlatBuffers": "onnxruntime/quantization/CalTableFlatBuffers",
    "onnxruntime.transformers": "onnxruntime/transformers",
    "onnxruntime.transformers.longformer": "onnxruntime/transformers/longformer"
  },
  "entry_points": {
    "console_scripts": [
      "onnxruntime_test = onnxruntime.tools.onnxruntime_test:main"
    ]
  }
}

@thatch
Copy link
Member

thatch commented Feb 9, 2022

A test would be nice, even if it's invalid python (unbound local x=x).

@amyreese amyreese changed the title More robust evaluation for self-referential names More robust evaluation of assignments Feb 9, 2022
@amyreese
Copy link
Member Author

amyreese commented Feb 9, 2022

Running this on onnxruntime now produces "version": "+cpu", which I don't like as much as "??" for this specific instance—but it's something I guess?—but would also correctly allow evaluating empty strings, where the original rev of this PR would have discarded x = '' and returned "??" instead.

@thatch
Copy link
Member

thatch commented Mar 2, 2022

Seems to produce issues when redefining builtins; see https://gist.github.com/thatch/ebc427075958a2338b7b7d712f77b01e for a simplified version of one that is copypasted over many popular projects' setup.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infinite recursion parsing setup.py from onnxruntime
2 participants