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

argparse_flags.ArgumentParser does not accept --flagfile if there are no flags #199

Closed
jacky8hyf opened this issue Sep 2, 2022 · 3 comments · Fixed by #260
Closed

argparse_flags.ArgumentParser does not accept --flagfile if there are no flags #199

jacky8hyf opened this issue Sep 2, 2022 · 3 comments · Fixed by #260

Comments

@jacky8hyf
Copy link

Minimum repro:

# foo.py
import absl.flags.argparse_flags
import argparse
# absl.flags.DEFINE_string("do_not_use_flagfile_hack", "", "") # A
parser = absl.flags.argparse_flags.ArgumentParser()
parser.add_argument("--foo")
parser.parse_args()
echo "--foo" > flags.txt
foo.py --flagfile=flags.txt

The error is:

foo.py: error: unrecognized arguments: --flagfile=flags.txt

The command would work if I uncomment line A.

It appears that ArgumentParser only accepts --flagfile if there are some DEFINE_'d strings. This is because of this line:

https://github.com/abseil/abseil-py/blob/main/absl/flags/argparse_flags.py#L156

    if self._inherited_absl_flags: # <---
      # Handle --flagfile.
      # Explicitly specify force_gnu=True, since argparse behaves like
      # gnu_getopt: flags can be specified after positional arguments.
      args = self._inherited_absl_flags.read_flags_from_files(
          args, force_gnu=True)

--flagfile is only accepted if bool(self._inherited_absl_flags), which is False if there are no DEFINE_d strings.

@yilei
Copy link
Contributor

yilei commented Sep 2, 2022

Thanks for the bug report and analysis! You are right, the code should check if self._inherited_absl_flags is not None instead. I'll try that.

@jacky8hyf
Copy link
Author

jacky8hyf commented Oct 30, 2023

Hi @yilei are there any updates?

@yilei
Copy link
Contributor

yilei commented Oct 31, 2023

@jacky8hyf Thanks for the ping! I picked this up again now.

copybara-service bot pushed a commit that referenced this issue Oct 31, 2023
…`None` instead of implicit bool.

The documented contract is to skip absl flags when `inherited_absl_flags` is passed `None`. A `FlagValues` instance without any flags evaluates to `False` and shouldn't let it skip.

PiperOrigin-RevId: 578025536
copybara-service bot pushed a commit that referenced this issue Oct 31, 2023
…`None` instead of implicit bool.

The documented contract is to skip absl flags when `inherited_absl_flags` is passed `None`. A `FlagValues` instance without any flags evaluates to `False` and shouldn't let it skip.

PiperOrigin-RevId: 578025536
copybara-service bot pushed a commit that referenced this issue Oct 31, 2023
…`None` instead of implicit bool.

The documented contract is to skip absl flags when `inherited_absl_flags` is passed `None`. A `FlagValues` instance without any flags evaluates to `False` and shouldn't let it skip.

PiperOrigin-RevId: 578025536
copybara-service bot pushed a commit that referenced this issue Oct 31, 2023
…`None` instead of implicit bool.

The documented contract is to skip absl flags when `inherited_absl_flags` is passed `None`. A `FlagValues` instance without any flags evaluates to `False` and shouldn't let it skip.

PiperOrigin-RevId: 578025536
copybara-service bot pushed a commit that referenced this issue Oct 31, 2023
…`None` instead of implicit bool.

The documented contract is to skip absl flags when `inherited_absl_flags` is passed `None`. A `FlagValues` instance without any flags evaluates to `False` and shouldn't let it skip.

PiperOrigin-RevId: 578025536
copybara-service bot pushed a commit that referenced this issue Oct 31, 2023
…ne` instead of implicit bool.

The documented contract is to skip absl flags when `inherited_absl_flags` is passed `None`. A `FlagValues` instance without any flags evaluates to `False` and shouldn't let it skip.

PiperOrigin-RevId: 578025536
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants