Skip to content

Migrate from optparse to argparse #208

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

Merged

Conversation

mandolaerik
Copy link
Contributor

  • Shut up linter
  • Remove the --illegal-attrs flag
  • Remove --illegal-attrs flag
  • Remove --version, which was ignored
  • Migrate from optparse to argparse

@syssimics
Copy link
Contributor

Verification #12224557: pass

@syssimics
Copy link
Contributor

Verification #12227474: pass

Copy link
Contributor

@lwaern-intel lwaern-intel left a comment

Choose a reason for hiding this comment

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

Undeniably uncontroversial

'--no-dep-phony', dest = "no_dep_phony", action = 'store_true',
help = 'With --dep, avoid addition of a phony target for each'
parser.add_argument(
'--no-dep-phony', action='store_true',
Copy link
Contributor

Choose a reason for hiding this comment

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

So with no explicit dest the parser will parse --no-dep-phony, and turn it into no_dep_phony when genning the options? Huh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's standard practice for argparse. Adn add_argument('-f', '--foo-bar', ...) stores it in foo_bar.

@@ -2392,3 +2392,7 @@ class PINT1(PortingMessage):
types, then the value of the variable becomes 1, whereas for
`int1` in DML 1.4 the value is -1."""
fmt = "Change int1 to uint1"

warnings = {name: cls for (name, cls) in globals().items()
Copy link
Contributor

@lwaern-intel lwaern-intel Sep 22, 2023

Choose a reason for hiding this comment

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

Just to double-check that I remember the semantics of globals() -- does it return the accessible vars defined in the current module only, or does it return the vars accessible at the top-level of the current module, no matter where they are defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somewhere inbetween, it seems -- globals() returns a dict that includes some builtin symbols like __name__, __loader__ and __builtins__, but not others like __import__.

We want to distinguish between proxy attributes and proxy interface ports
Note that compared to generate_attribute_common, the condition used by
need_port_proxy_attr adds a precondition that the bank/port must be on top
level. The change is still a pure refactoring since generate_attribute_common
is only called in DML 1.2, which requires banks and ports to be on top level.
@mandolaerik mandolaerik force-pushed the pr/migrate-from-optparse-to-argparse branch from e54543f to cf988af Compare September 26, 2023 09:41
@syssimics
Copy link
Contributor

Verification #12247784: pass

@mandolaerik mandolaerik merged commit cd3368b into intel:main Sep 26, 2023
@mandolaerik mandolaerik deleted the pr/migrate-from-optparse-to-argparse branch September 26, 2023 10:20
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.

3 participants