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

Fix #164, dylink import behavior for names that start with "_" #188

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aaaaalbert
Copy link
Contributor

This PR proposes tests to highlight #164, and also a fix that makes dylink's import functions behave like Python's. From the commit message of the ``fix'' part,

  • dy_import_module_symbols (akin to Python's from MODULE import *)
    now correctly skips names that start with an underscort ("_"), and
  • dy_import_module (akin to Python's import MODULE) now does
    correctly import these names.

These tests try to illustrate Python's behavior on `import`ing or
`from MODULE import *`ing a name that starts with an underscore
(``_'') character, and check whether `dylink`'s corresponding
functions `dy_import_module` and `dy_import_module_symbols`
perform like their Python counterparts.

The expected behavior is to
* do import `module._underscorename`, but
* don't add `_underscorename` via `from module import *`

Alas, `dylink` doesn't conform currently: SeattleTestbed#164 .
This patch makes `dylink.r2py`'s import functions behave more
like their Python counterparts. Specifically,
* `dy_import_module_symbols` (akin to Python's `from MODULE import *`)
  now correctly *skips* names that start with an underscort ("_"), and
* `dy_import_module` (akin to Python's `import MODULE`) now *does*
  correctly import these names.

The new test cases supplied by the parent commit,
bb285e4 ,
thus all pass now.
@aaaaalbert
Copy link
Contributor Author

See also the relevant post-merge test results for these tests (on a test branch of mine ATM), https://travis-ci.org/aaaaalbert/seattlelib_v2/jobs/155357118#L372-L375

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant