-
Notifications
You must be signed in to change notification settings - Fork 58
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
ENH: Allow attr_fields to exclude fields #426
Conversation
9cf9590
to
0b217c6
Compare
Codecov Report
@@ Coverage Diff @@
## master #426 +/- ##
==========================================
- Coverage 82.45% 82.39% -0.06%
==========================================
Files 19 19
Lines 3859 3846 -13
Branches 1054 1048 -6
==========================================
- Hits 3182 3169 -13
Misses 487 487
Partials 190 190
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
0b217c6
to
c840917
Compare
else: | ||
raise Exception("not implemented (collect_additional_output)") | ||
for fld in attr_fields(self, exclude_names=("return_code", "stdout", "stderr")): | ||
if fld.type not in [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm actually not sure if we should be checking this here.. but can think about this in a different pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make an issue and expand on your reasoning here? I agree that we should keep this PR focused.
lgtm |
Acknowledgment
Types of changes
Summary
It is fairly common for calls to
attr_fields
to be followed by a filter for specific names that get ignored. This adds an exclusion list to the call to simplify the loops.Checklist
(we are using
black
: you canpip install pre-commit
,run
pre-commit install
in thepydra
directoryand
black
will be run automatically with each commit)