Skip to content

Remove exec magic from sasdata.quantities.units#204

Open
rprospero wants to merge 3 commits into
refactor_24from
units_without_exec
Open

Remove exec magic from sasdata.quantities.units#204
rprospero wants to merge 3 commits into
refactor_24from
units_without_exec

Conversation

@rprospero
Copy link
Copy Markdown
Contributor

My previous work on refactoring out the units module depended heavily on writing a module at run time and then running the code through exec. While this removed the need to keep _build_tables.py and units.py in sync, it was also very ugly code that could be very frightening to edit.

This PR produces a modified version that edits an exiting units module instead of trying to write a new one from scratch.

  • cleaner and easier to read than old code
    • No use of StringIO
    • No use of exec
    • Easier to follow data paths (e.g. the fid.write(f" '{name}': {name}") pattern obscures the actual value in the dictionary).
  • units module code isn't run if units module isn't called (the exec version would build the units module the moment someone imported sasdata.quantities instead of sasdata.quantities.units)

Again providing guidance:

  • Everything before line 899 of units.py was just moved directly from __init__.py
  • The variable name this was chosen to match convention from the Javascript community. I had also considered using self, which is more Pythonic, but had the potential to be confusing, since it is usually attached to a class method.
  • The remaining code is still a modification of units.py. However, values are defined directly and moved into the module through setattr.

codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@rprospero rprospero force-pushed the units_without_exec branch from 10a93d9 to e732111 Compare May 14, 2026 16:18
codescene-delta-analysis[bot]

This comment was marked as outdated.

@rprospero rprospero requested a review from DrPaulSharp May 18, 2026 09:50
@DrPaulSharp DrPaulSharp force-pushed the refactor_24 branch 3 times, most recently from 90413d7 to dd691e0 Compare May 18, 2026 17:07
@rprospero rprospero force-pushed the units_without_exec branch from e732111 to 9b74726 Compare May 26, 2026 10:01
codescene-delta-analysis[bot]

This comment was marked as outdated.

@rprospero rprospero force-pushed the units_without_exec branch from 9b74726 to 052581e Compare May 26, 2026 10:03
Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No quality gates enabled for this code.

See analysis details in CodeScene

Quality Gate Profile: Custom Configuration
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

Copy link
Copy Markdown
Contributor

@DrPaulSharp DrPaulSharp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good. I have one question, you noted that "Everything before line 899 of units.py was just moved directly from __init__.py". There is also code there that is copied from _units_base.py - did you intend to move that code, or is the copy correct?

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.

2 participants