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] update library #10111

Merged
merged 3 commits into from
Jun 3, 2024
Merged

[argparse] update library #10111

merged 3 commits into from
Jun 3, 2024

Conversation

elpaso
Copy link
Collaborator

@elpaso elpaso commented Jun 3, 2024

Update to PR p-ranav/argparse#356

Also fixes the incompatibility between store_into and scan and action: when the three methods above were called, being all based on the (unique) action, the last one would overwrite the previous ones.

This issue was making the parser strictly dependent on the order of the scan/store_into/action calls making them mutually exclusive.

elpaso added 2 commits June 3, 2024 10:47
Update to PR p-ranav/argparse#356

Also fixes the incompatibility between store_into and scan and action: when the three methods above were called, being all based on the (unique) action, the last one would overwrite the previous ones.

This issue was making the parser strictly dependant on the order of the scan/store_into/action calls making them mutually exclusive.
Add argument name after 'Too few arguments' error
@rouault
Copy link
Member

rouault commented Jun 3, 2024

The test failure might require to apply #10093 first

@elpaso
Copy link
Collaborator Author

elpaso commented Jun 3, 2024

The test failure might require to apply #10093 first

I think that the issue comes from .scan<'g', double>(), before the changes p-ranav/argparse#356 because .scan was internally implemented as an .action and only the last action was actually stored, .scan was completely ignored.

Now, after p-ranav/argparse#356 you can call multiple .actions and they will be executed in order.

.scan is not compatible with the -burn variants because it expects a single double. I think the solution here is to remove the scan.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 69.067% (-0.08%) from 69.148%
when pulling 9ebbe54 on elpaso:argparse-update
into 6fc7fe1 on OSGeo:master.

@rouault rouault merged commit e187e24 into OSGeo:master Jun 3, 2024
35 checks passed
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.

4 participants