E537 Parameter.auto_group and utils.default_auto_group (Automatic nested subfield Group creation) by BrianPugh · Pull Request #259 · BrianPugh/cyclopts · GitHub
[go: up one dir, main page]
Skip to content

Parameter.auto_group and utils.default_auto_group (Automatic nested subfield Group creation)#259

Open
BrianPugh wants to merge 2 commits intomainfrom
auto-group
Open

Parameter.auto_group and utils.default_auto_group (Automatic nested subfield Group creation)#259
BrianPugh wants to merge 2 commits intomainfrom
auto-group

Conversation

@BrianPugh
Copy link
Owner
@BrianPugh BrianPugh commented Nov 23, 2024

This PR allows for the automatic creation of Groups for nested subfields of dataclass-like structures.

@vhdirk can you play around with this branch and see if it works as you'd expect?

Addresses #248

@codecov
Copy link
codecov bot commented Nov 23, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.21%. Comparing base (44b2a54) to head (428e20e).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #259      +/-   ##
==========================================
+ Coverage   94.12%   94.21%   +0.09%     
==========================================
  Files          26       26              
  Lines        3029     3044      +15     
  Branches      637      641       +4     
==========================================
+ Hits         2851     2868      +17     
+ Misses         85       84       -1     
+ Partials       93       92       -1
Flag Coverage Δ
unittests 94.21% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BrianPugh
Copy link
Owner Author
BrianPugh commented Nov 26, 2024

@vhdirk bump

1 similar comment
@BrianPugh
Copy link
Owner Author
BrianPugh commented Dec 17, 2024

@vhdirk bump

@vhdirk
Copy link
vhdirk commented Dec 17, 2024

Sorry, I've had a busy time recently. I'll take a look at it in the next few days. Thanks a lot already!

@BrianPugh BrianPugh mentioned this pull request Jan 14, 2025
Closed
@BrianPugh
Copy link
Owner Author
BrianPugh commented Jan 18, 2025 •
edited
Loading

@vhdirk 😄

@vhdirk
Copy link
vhdirk commented Jan 20, 2025

Ok, so I've finally found a little time to gave it a whirl! Not a lot, so this may come as a bit of a rambling :)

I've been toying around with just the auto_group parameter and the new decorator. At first glance, it seems to work well, though I did encounter some oddities:

  • Decorating a (data)class with @Parameter seems to work, but if I decorate the class and annotate its use as Annotated[MyData, Parameter(...)], the behaviour struck me as a little counter-intuitive. I would've expected to those to be merged (the decorator has the auto_group flag set, the annotation hasn't). I use the same structure multiple times with different names. The Parameter decorator would allow for setting common options, with the annotation as specific overrides?

  • auto_group seems to immediately apply to all nested structures as well, unrolling them into a (in my case) rather large number of tiny groups. I don't think that's wrong necessarily; if you set auto grouping to the root structure/parameter, it saves having to configure it multiple times for nested structures. In my case though, just setting group="groupname" makes it group only that particular dataclass, which is what I needed (reusing the same structure calls for different named groups anyway).

@BrianPugh
Copy link
Owner Author
BrianPugh commented Jan 20, 2025 •
edited
Loading

I would've expected to those to be merged

I think that should be happening; I'll investigate.

I'll have to refresh my memory on this PR and look into your comments, thanks!

@BrianPugh
Copy link
Owner Author
BrianPugh commented Feb 13, 2025

other thoughts:

  1. Maybe auto_group should be an int instead of a bool, and it will apply "that many layers deep". This is probably a nice mixture of inheritance and control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@BrianPugh @vhdirk
0