BrandonFors/pandas-github-issues huggingface dataset:
Document Tips for Debugging C Extensions
State: closed

Quick notes in case they are helpful to others. Can move to the Wiki as well if we don't want in the standard docs

Labels:
name: Docs

Comments:
Hello @WillAyd! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-12-09 00:49:35 UTC
Any other comments here? Otherwise plan on merging in a few days

One question, no objections.
any movement here?
Been tight on time lately so closing this temporarily to clean the queue - will come back to this at a later date
Getting back around to this. Since there was some confusion around gdb vs lldb I added a blurb about which one to choose when. I also made it so all of the steps are shown for both debuggers
thanks @WillAyd
BENCH: implement asvs for ints_to_pydatetime
State: closed


Labels:
name: Benchmark
description: Performance (ASV) benchmarks

Comments:
Hello @jbrockmendel! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-07-01 22:28:53 UTC
thanks
TST: update gbq service account key
State: closed

Re-enable gbq integration tests.

- [ ] closes #34779
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
Build currently running in my personal Travis config at https://travis-ci.org/github/tswast/pandas/builds/704081779

I need additional permissions in order to create the encryption key environment variables on the pandas-dev Travis config.
@tswast does this run?
@jreback It [runs on my fork](https://travis-ci.org/github/tswast/pandas/builds/704081779), but still having trouble with encrypting the keys for use in the pandas-dev org.

```
$ travis encrypt-file -r pandas-dev/pandas $GBQ_JSON_FILE travis_gbq.json.enc -f

encrypting pandas-gbq-tests-e842e94bcd25.json for pandas-dev/pandas
storing result as travis_gbq.json.enc
storing secure env variables for decryption
resource not found ({"error":"Couldn't find repository"})
```

I can now see the pandas project under "my repositories" in Travis. But maybe it's still not working because I can't see the "Settings" menu like I can on my fork.

<img width="834" alt="Screen Shot 2020-07-06 at 9 45 32 AM" src="https://user-images.githubusercontent.com/247555/86606422-9461cf80-bf6d-11ea-85e8-1c51fd0f66a4.png">

<img width="836" alt="Screen Shot 2020-07-06 at 9 47 25 AM" src="https://user-images.githubusercontent.com/247555/86606564-c6733180-bf6d-11ea-9076-906a8a5399e8.png">


Looks like write access is needed in order to update settings (including adding encryption keys?). 😕 https://docs.travis-ci.com/user/travis-ci-for-private/#who-has-access-to-the-builds
I was able to add the required keys to Travis. Since Travis doesn't allow the use of keys from forks, I also pushed this branch https://github.com/pandas-dev/pandas/tree/issue34779-enable-gbq-tests, which can be deleted once verifying the build at https://travis-ci.org/github/pandas-dev/pandas/builds/705824144
ok let's try this.
CLN: Removed unreached else block GH33478
State: closed

Unreached else block in method _wrap_applied_output was removed.

Local tests passed before and after change

- [ ] xref #33478 (Not fully closed, this addresses 1 part)
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Groupby
name: Clean

Comments:
> closes #33478 (Not fully closed, this addresses 1 part)

Can you change "closes" to "xref"?
The `Windows py37_np18` shows a failure (`pandas/tests/base/test_unique.py::test_unique_bad_unicode[idx_or_series_w_bad_unicode0]`), although if I check out your pull request and run it myself it passes (also, none of the functions in `pandas/tests/base/test_unique.py` hit this code path), so am pretty sure the CI failure is unrelated
Oh, weird! I ran the full test suite (test_fast.sh) before committing and no failures for me either. Let me know if you need me to do anything else
thanks @FollowTheProcess 
Thanks both for the help. I hope to contribute more in the future!
Fix numpy warning
State: closed


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
Various failures like

```
=================================== FAILURES ===================================

_______________ TestIndex.test_isin_nan_common_object[nan0-nan2] _______________

[gw0] linux -- Python 3.8.3 /home/travis/miniconda3/envs/pandas-dev/bin/python

self = <pandas.tests.indexes.test_base.TestIndex object at 0x7ff81c8b2e80>

nulls_fixture = nan, nulls_fixture2 = nan

    def test_isin_nan_common_object(self, nulls_fixture, nulls_fixture2):

        # Test cartesian product of null fixtures and ensure that we don't

        # mangle the various types (save a corner case with PyPy)

    

        # all nans are the same

        if (

            isinstance(nulls_fixture, float)

            and isinstance(nulls_fixture2, float)

            and math.isnan(nulls_fixture)

            and math.isnan(nulls_fixture2)

        ):

>           tm.assert_numpy_array_equal(

                Index(["a", nulls_fixture]).isin([nulls_fixture2]),

                np.array([False, True]),

            )
```

which I can't look into at the moment. I'll just remove the duplicate key.
No fastcall attribute in POWER platform
State: closed

Compiling the development branch of pandas fail in AIX. 

pandas/_libs/src/ujson/lib/ultrajsonenc.c:397:1: error: ‘fastcall’ attribute directive ignored [-Werror=attributes]
 Buffer_AppendShortHexUnchecked(char *outputOffset, unsigned short value) {
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pandas/_libs/src/ujson/lib/ultrajsonenc.c:726:59: error: ‘fastcall’ attribute directive ignored [-Werror=attributes]
                                                           char *end) {
                                                           ^~~~
cc1: all warnings being treated as errors


The fastcall attribute is not available in POWER platform also. 



Labels:
name: Build
description: Library building on various platforms

Comments:
FWIW I would be OK with removing this altogether. AFAIK it's only for i386 platforms which we don't really care to optimize at this point
> FWIW I would be OK with removing this altogether. AFAIK it's only for i386 platforms which we don't really care to optimize at this point

Okay, so shall i remove it altogether in this PR ?
Yea let's go ahead and remove it. If you can run the JSON benchmarks after than and post the results would help confirm
Not sure how to run those JSON benchmarks
Check this guide:


https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#running-the-performance-test-suite

You can use the -b json switch at the end to limit the results to that module
looks fine @WillAyd merge when good
@ayappanec can you remove all occurrences of this macro instead of the empty define?
[JSON_Benchmarks.txt](https://github.com/pandas-dev/pandas/files/4884421/JSON_Benchmarks.txt)
Attaching the JSON benchmarks results here
@ayappanec can you run vs the master commit, e.g. https://pandas.pydata.org/docs/dev/development/contributing.html#running-the-performance-test-suite, you need `asv continuous`
> @ayappanec can you run vs the master commit, e.g. https://pandas.pydata.org/docs/dev/development/contributing.html#running-the-performance-test-suite, you need `asv continuous`

Looks like hdf5 needs to be installed . I never encountered hdf5 before
Installed HDF5 but now it fails with llvmlite build.

           raise RuntimeError("unsupported platform: %r" % (sys.platform,))
       RuntimeError: unsupported platform: 'aix6'

Looks like the asv benchmark testing needs some pre-requisites which has compilation issues in AIX .
Any update on getting this merged ?
thanks @ayappanec 
@WillAyd @jreback Thanks for the support.
BUG: DataFrameGroupBy.__getitem__ fails to propagate dropna
State: closed

- [x] closes #35014 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Labels:
name: Groupby

Comments:
A note - in addition to the missing values handling, his PR also fixes a version of #14466 in the `SeriesGroupBy` version of transform

I solved it similarly to #12559
Hello @arw2019! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-08-07 19:26:30 UTC
Ready to go modulo any comments
@TomAugspurger what do you think? This aims to resolve the `SeriesGroupBy.__getitem__` bug you pointed out a few weeks ago 
I think https://github.com/pandas-dev/pandas/pull/35444 is a more general soln here. 
@jreback: Unfortunately my PR is not sufficient here. The root issue lies with the use of a dictionary for `self.indices` within the `_GroupBy` class. Trying to make a key be `None` (or `np.nan` or `pd.NaT`) causes issues. AFAICT, dropna is never used once the GroupBy object is created.

I think this isn't an issue with propagating `dropna`, but rather with the SeriesGroupBy transform function itself. Somehow DataFrameGroupBy avoids these issues, although I need to spend more time looking through the code to understand how.
you are making a lot of changes here, pls try to simplify.
> you are making a lot of changes here, pls try to simplify.

@jreback Ok!

I redid the solution by copying the logic in `DataFrameGroupBy._transform_general` which is very similar & avoids this problem (thanks @rhshadrach for the suggestion)

thanks @arw2019 
thanks @jreback for reviewing
TST: Test for groupby transform on categorical column
State: closed

Added test for groupby transform on categorical column

- [x] closes #29037 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


Labels:
name: Testing
description: pandas testing functions or related to the test suite

Comments:
Thanks @erfannariman
Add test apply dtype
State: closed

- [x] closes #31466
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Groupby

Comments:
Looks great. Thanks @grahamwetzler
PERF: Timestamp.normalize
State: closed

The perf is actually a wash here, verging on slightly negative.  The really worthwhile thing is the improved asvs.

The big perf gain is being split into a separate PR that optimizes _maybe_convert_value_to_local to the tune of 40%.

Labels:
name: Performance
description: Memory or execution speed performance
name: Timezones
description: Timezone data dtype

Comments:
sure pls rebase
rebased+green
DOC: Remove mypy from pre commit
State: closed

- [x] closes #34902
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


Labels:
name: Code Style
description: Code style, linting, code_checks
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
are we sure we really want to do this? whats going to happen is that mypy issues will just happen on the CI, which is much later in the process.
I don't have a strong point of view but the current usage of mypy is very limited in scope and can in a lot of cases yield a different result than CI anyway. I do think a full mypy run pre-push would be a good compromise instead of a partial run every commit just not sure how feasible that is
This issue is still a thing and for now I solve this with `--no-verify` at commit, which is not desirable either, since it will skip the other tests as well. Can we maybe add the "needs discussion"  tag to have some other opinions as well? @jreback @WillAyd 

Just for my understanding, we can't solve the error messages which are occuring now? That would be the most desirable solution right?
> we can't solve the error messages which are occuring now?

cc @simonjayhawkins could this be resolved by ensuring that we have the same version of mypy run locally as on the CI?
> > we can't solve the error messages which are occuring now?
> 
> cc @simonjayhawkins could this be resolved by ensuring that we have the same version of mypy run locally as on the CI?

AFAIK mypy runs fine locally with mypy 0.730 (although with other packages adding types e.g. pytest, probably need to make sure local environment matches environment on ci). for ci the dev enviroment is used. i.e. environment.yaml

as far as the pre commit is concerned, I'm on Windows and didn't seem to work out of the box and not spent any time looking any further.
@simonjayhawkins I saw that there were some changes concerning mypy, is PR still relevant? 

> @simonjayhawkins I saw that there were some changes concerning mypy, is PR still relevant?

I'm not really aware of the issues here, as I don't use pre commit, so I'm not the right person to ask/comment.
Let's just remove it. We can try again once it's more mature.
Assorted ujson Cleanups
State: closed

I think it would be great if we could move the block iteration out of JSON as it has generic functionality that we could use down in C. This isn't it, but a small set of cleanups I noticed while looking at that

Labels:
name: IO JSON
description: read_json, to_json, json_normalize
name: Clean

Comments:
lgtm. but i would run thru valgrind to make sure we dont' have memory leaks. (but can be future).
ENH: support index=True/False keyword for io.sql.get_schema
State: closed

- [x] closes #9084
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Enhancement
name: IO SQL
description: to_sql, read_sql, read_sql_query
name: Stale

Comments:
Sorry, a couple issues with the docstring: https://github.com/pandas-dev/pandas/pull/35063/checks?check_run_id=842449220#step:10:9. Do you have time to fix those here? `python scripts/validate_docstring.py pandas.io.sql.get_schema` should print all the issues.
> get_schema is meant to be user-facing?

That's not entirely clear, see https://github.com/pandas-dev/pandas/issues/9960


I think we should just consider it public and document it as is.
@VirosaLi From the above looks like this might be ready to go? Maybe merge master again?
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
Hello @fangchenli! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-12-08 04:46:02 UTC
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
@fangchenli - still interested in working on this or shall we close?
closing as stale. if you want to continue, pls ping and can re-open.
Fixed #34859: Added support for '0' and '1' in BooleanArray._from_sequence_of_strings method
State: closed

- [ ] closes #34859 
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: IO CSV
description: read_csv, to_csv
name: NA - MaskedArrays
description: Related to pd.NA and nullable extension arrays

Comments:
> Can you also add a test that this works for `read_csv`?

@TomAugspurger Can you please suggest me where would this test be appropriate to add.?
In pandas/tests/io/parser/test_dtypes.py, around the `test_boolean_dtype`
test.

On Wed, Jul 1, 2020 at 1:29 AM SanthoshBala18 <notifications@github.com>
wrote:

> Can you also add a test that this works for read_csv?
>
> @TomAugspurger <https://github.com/TomAugspurger> Can you please suggest
> me where would this test be appropriate to add.?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/35061#issuecomment-652219832>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOIX3V46XPEEM3NED553RZLJUVANCNFSM4OMGAW3Q>
> .
>

@TomAugspurger Thanks, I have added the tests. Any reason why the CI build would fail.?
Probably just some instability in conda / cloudflare. I've restarted the failed builds.
thanks @SanthoshBala18 very nice
Don't we need some documentation for this?
for what reason? bool already does this and is pretty standard 

this is just compatibility
Fix issue #31708 Series.astype(str, skipna=True) vanished in the 1.0 release
State: closed

- [ ] closes #31708
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Labels:
name: Bug
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
> not sure you understood the original issue
> we are not adding this kwarg

skipna is not passed as a kwarg, now it is valid keyword for astype
Right I'm not sure - would you delete this argument and have skipna=True in astype_nansafe? Then it would be API breaking change 
@t0ma-sz i don't think its worth restoring this at all, rather fixing #25353 would be useful.
closing as wont fix - the underlying cause should be addressed rather than adding back unsupported api
DOC: Alligned docstring for ignore_index in append
State: closed

- [x] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Docs

Comments:
While working on #34932 I found out that `DataFrame.append` and `Series.append` were the only methods with `ignore_index` where the docstring was different.
Makes sense - ping on green
Thanks @erfannariman
TST: Adds CategoricalIndex DataFrame from_records test (GH32805)
State: closed

- [x] closes #32805
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

The test checks to make sure `from_records` loads a `DataFrame` with a `CategorigalIndex` as shown in #32805.

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Categorical
description: Categorical Data Type
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
Hello @jgulian! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-07-13 22:30:14 UTC
Looks good. Just need to run isort on the affected file to get the right import order

@mroeschke merge if looks good to you
@jgulian Could you run `isort pandas/tests/frame/test_constructors.py`?

After that LGTM 
thanks @jgulian
POC: make ccalendar.pyx valid python
State: closed

Proof of concept for making some of our cython files into valid python\*, which would allow us to lint and maybe (cc @simonjayhawkins thoughts?) use mypy

\* strictly speaking, we need to do a find-and-replace of "cimport" -> "import" to get all the way to valid python.

Labels:

Comments:
cool. i've been looking into this but not got very far since I need to better understand cython first.

One thing that I have thought while investigating is that it may be possible to move some non performance related code (not looping) out of the cython. 

for instance in array_with_unit_to_datetime in pandas/_libs/tslib.pyx, there seems to be logic to determine whether to iterate. I wonder whether code like this could be in python so that the cython code is just the iteration. (I think some more code that could be pure python being added in #35027?)

of course the approach in this PR helps us to keep relevant code together, while still allowing linting and mypy checking. 


Is this style documented by Cython?
> Is this style documented by Cython?

It's documented as "pure-python mode".

> One thing that I have thought while investigating is that it may be possible to move some non performance related code (not looping) out of the cython.

As long as it doesn't break the self-contained-ness of tslibs and near-self-contained-ness of _libs, I'm all for this.

@simonjayhawkins is this something where we get a non-trivial benefit even if we only get a handful of modules?  This is one of the easiest modules to do this on.  In particular pure-python mode doesn't support `const int64_t[:]` or `ndarray[int64_t]` etc.
Closing to clear the queue since this is just a proof of concept.  Can re-open if @simonjayhawkins thinks there is benefit to getting some but not all of these files python-valid
Fix issue #29837: added test case for aggregation with isnan
State: closed

- [x] closes #29837
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Groupby
name: Categorical
description: Categorical Data Type

Comments:
Hello @biddwan09! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-06-29 04:30:55 UTC
@biddwan09 thanks for the PR. FYI Linux py37_np_dev test failures unrelated, xref #35041


Hi @simonjayhawkins let me know if any other change is required
Thanks @biddwan09
BUG: fix read_excel error for header=None and index_col as list #31783
State: closed

- [x] closes #31783
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Bug
name: IO Excel
description: read_excel, to_excel

Comments:
/azp run
<samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
Thanks for the review guys. Should be fine now @jreback 
thanks @rjfs