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

BUG/DOC: Add documentation in types/common.py #15941

Merged
merged 3 commits into from
Apr 7, 2017

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Apr 7, 2017

Adds documentation for all internal functions in types/common.py

In addition, caught a bug in which some functions calling _get_dtype were not catching the TypeError. Documented those functions along the way too.

Partially addresses #15895.

@gfyoung gfyoung force-pushed the pandas-types-doc branch 2 times, most recently from 3ac9d08 to 986d8b6 Compare April 7, 2017 17:36
dtype = _get_dtype(arr_or_dtype)
return dtype.kind in ('O', 'S', 'U') and not is_period_dtype(dtype)
"""
Check whether the provided array or dtype is of the string dtype.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments here: #15585

maybe add a reference / comments

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sounds good. Done.

@gfyoung gfyoung changed the title DOC: Add documentation in types/common.py BUG/DOC: Add documentation in types/common.py Apr 7, 2017
@jreback jreback added the Docs label Apr 7, 2017
@codecov
Copy link

codecov bot commented Apr 7, 2017

Codecov Report

Merging #15941 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15941      +/-   ##
==========================================
+ Coverage   90.98%   90.99%   +<.01%     
==========================================
  Files         145      145              
  Lines       49556    49565       +9     
==========================================
+ Hits        45090    45101      +11     
+ Misses       4466     4464       -2
Flag Coverage Δ
#multiple 88.76% <100%> (ø) ⬆️
#single 40.56% <37.5%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/types/common.py 94.48% <100%> (+0.83%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d90a43...aa9ec1b. Read the comment docs.


Returns
-------
is_orderable_exc : Whether or not the exception raised is an
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think better to use say:

Returns
--------
boolean : .......

(for all of these)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, done.

#
# No exception should be raised.

assert not com.is_string_dtype(None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add all of these (use parametrize / construct them)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, done.



get_dtype_funcs = [
(com.is_dtype_equal, 2),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I meant was to add all of the validation functions.

(In [3]: [ f for f in dir(com) if f.startswith('is_') and f.endswith('dtype')]
Out[3]: 
['is_any_int_dtype',
 'is_bool_dtype',
 'is_categorical_dtype',
 'is_complex_dtype',
 'is_datetime64_any_dtype',
 'is_datetime64_dtype',
 'is_datetime64_ns_dtype',
 'is_datetime64tz_dtype',
 'is_datetime_or_timedelta_dtype',
 'is_float_dtype',
 'is_floating_dtype',
 'is_int64_dtype',
 'is_int_or_datetime_dtype',
 'is_integer_dtype',
 'is_numeric_dtype',
 'is_object_dtype',
 'is_period_dtype',
 'is_signed_integer_dtype',
 'is_string_dtype',
 'is_string_like_dtype',
 'is_timedelta64_dtype',
 'is_timedelta64_ns_dtype',
 'is_unsigned_integer_dtype']

then is_dtype_equal should be a separate function to test (as its different)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could make this a nice fixture :>

Copy link
Member Author

@gfyoung gfyoung Apr 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the whole point was to check all of the functions that _get_dtype. The ones in the list currently are the only ones that do.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could make this a nice fixture :>

Perhaps, if it's needed 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, well we should check them all anyhow :>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Will do then.

The following functions were not catching
the TypeError raised by _get_dtype:

1) is_string_dtype
2) is_string_like_dtype
3) is_timedelta64_ns_dtype

Thus, when "None" was passed in, an
Exception was raised instead of returning
False, as other functions did.
@jreback jreback merged commit bc2fa16 into pandas-dev:master Apr 7, 2017
@jreback
Copy link
Contributor

jreback commented Apr 7, 2017

thanks @gfyoung

@gfyoung gfyoung deleted the pandas-types-doc branch April 8, 2017 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants