Remove and deprecate nfields #653
Conversation
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #653 +/- ##
==========================================
- Coverage 92.97% 92.97% -0.01%
==========================================
Files 30 30
Lines 4500 4484 -16
==========================================
- Hits 4184 4169 -15
+ Misses 316 315 -1
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
I had been thinking if we should entirely remove I have a hard time coming up with a use case where you'd only want to know the number of fields and nothing more. Usually you'd want to iterate over them and that's what What do you think? |
|
I was about to suggest the same. |
|
Thanks for the feedback on this. I have no strong opinion here. Just thought that it might be a nice convenience function. But I can also remove it if that is what we want to do. |
|
Looking at the small diff, it looks like we don't even use it much internally. |
This patch deprecates the internal function `nfields(dh)` in favor of `length(getfieldnames(dh))`. The reason is twofold: `nfields` where barely used, not even internally, and `nfields` was easy to confuse with the existing `Base.nfields` function that would still work, but behave quite differently. Closes #444.
fredrikekre
force-pushed
the
do/rename-nfields
branch
from
4fb3b0f
to
b7a79b7
Compare
Closes #444 .