BrandonFors/pandas-github-issues huggingface dataset:
BUG: merge_asof should be treated as a left join
State: closed

- [x] closes #33463
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

The fix broke one test (`test_merge_index_column_tz` in `test_merge_asof.py`) which expected the right index instead of the left index.

The default index selection does not work in case of asof and `left_index=True`. I had to catch this case here.

Labels:
name: Bug
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode

Comments:
@phofl is this still active? If so can you address @mroeschke comments / questions?
Hello @phofl! 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 2021-04-11 11:47:41 UTC
@WillAyd Yeah, for sure. Missed that completely, sorry.
@jreback 
Did you have something like this in mind?

I have a more general question about the ``_get_join_info`` part. The else part starting in line 884 is a bit weird in my opinion. The if condition ``if self.right_index`` is only reached in cases of inner, right and outer join. In case of a right join, we use the **left** index on purpose here. So the following code

```
import pandas as pd

index = pd.Index([1, 5, 10], name="test")
left = pd.DataFrame({"left": [1, 2, 3, 6, 7]}, index=[1, 2, 3, 6, 7])
right = pd.DataFrame({"right": ["a", "b", "c"], "right_time": [1, 4, 10]}, index=index)
result = pd.merge(left, right, right_index=True, left_on='left', how="right")
```

produces the left index. 

```
     left right  right_time
1.0     1     a           1
NaN     5     b           4
NaN    10     c          10
```

although I would expect the right index here. Is this assumption correct? In this case I would modify the function calls of ``_create_join_index`` in a way to produce the desired results and keep the correct behavior in case of outer and inner joins. There are a few open issues about this topic.
@jreback Could you maybe have a look?
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.
@jreback seems like all comments were addressed, could you take a look? Thanks!
can you merge master and move the note to 1.3 and will look.
Done
@phofl oldie, but if you can merge will take a look
@jreback merged, failure unrelated
ok let me look closer at this.
Generally this looks okay to me. Mind merging in master?
merged master
thanks @phofl very nice!
CLN: Removed duplicated test data
State: closed

Remove duplicated test data.

"test1.csv" & "tips.csv" are present in both "tests/io/parser/data" & "tests/io/data/csv".

Move the "*.csv.bz2" & "*.csv.gz" files since the s3_resource fixture in tests/io/conftest requires these.

Follow up from: https://github.com/pandas-dev/pandas/issues/29439

Think we should move all the data in io/parser/data/* into io/data/ so it can be re-used. Will save for follow up.

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

Comments:
Hey @alimcmaster1 

Is part of this already addressed in #34458?
> Hey @alimcmaster1
> 
> Is part of this already addressed in #34458?

Morning, thanks for pointing this out. I think we will just want to remove pandas/tests/data in that PR? Whereas here I was looking at io/parser/data and io/data directories here

@WillAyd you able to review this? Thanks! 
thanks @alimcmaster1 very nice
BLD/PERF: bump cython to 0.29.19 for searchsorted
State: closed

cython 0.29.19 fixed a bug that now allows us to use `cnp.PyArray_SearchSorted` to make `ndarray.searchsorted` into a C call.  As a demonstration, this PR makes that change in `normalize_i8_timestamps` and shaves about 12% off of it for the scalar case (the non-scalar case is not noticeably affected)

```
In [4]: ts2 = pd.Timestamp.now("US/Pacific")                                    
In [5]: %timeit ts2.normalize()                                                 
59.3 µs ± 197 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)   # <-- master
51.8 µs ± 362 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)   # <-- PR
```

Not a particularly big deal on its own, _but_ this makes viable passing `int64_t*` instead of `int64_t[:]`, which in turn makes it so we can skip a wrapping/unwrapping step in the scalar code _and_ ideally share code between Timestamp/DatetimeArray, Period/PeriodArray, Timedelta/TimedeltaArray.

Labels:

Comments:
@scoder any idea how to get a more helpful traceback?

```
multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/home/vsts/miniconda3/envs/pandas-dev/lib/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/home/vsts/miniconda3/envs/pandas-dev/lib/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/vsts/miniconda3/envs/pandas-dev/lib/python3.7/site-packages/Cython/Build/Dependencies.py", line 1249, in cythonize_one_helper
    return cythonize_one(*m)
  File "/home/vsts/miniconda3/envs/pandas-dev/lib/python3.7/site-packages/Cython/Build/Dependencies.py", line 1225, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: pandas/_libs/tslibs/conversion.pyx
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "setup.py", line 792, in <module>
    setup_package()
  File "setup.py", line 762, in setup_package
    ext_modules=maybe_cythonize(extensions, compiler_directives=directives),
  File "setup.py", line 538, in maybe_cythonize
    return cythonize(extensions, *args, **kwargs)
  File "/home/vsts/miniconda3/envs/pandas-dev/lib/python3.7/site-packages/Cython/Build/Dependencies.py", line 1093, in cythonize
    result.get(99999)  # seconds
  File "/home/vsts/miniconda3/envs/pandas-dev/lib/python3.7/multiprocessing/pool.py", line 657, in get
    raise self._value
Cython.Compiler.Errors.CompileError: pandas/_libs/tslibs/conversion.pyx
```
No progress debugging this, we can wait for 3.0 anyway.  closing
@jbrockmendel the main part of that stack trace seems missing. There should be more output somewhere else in the build log, probably hidden by the parallel build. A serial build would also bring the output in order again.
BUG: Fix read_json shape error on multi-indexed DF/SR with orient='split' #4889
State: closed


- [ ] 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: Bug
name: IO JSON
description: read_json, to_json, json_normalize
name: Stale

Comments:
As stated previously I think the issue is with the DataFrame constructor and not anything in the JSON code - @LucasG0 is that something you'd like to investigate?
Yes, I'd like to investigate when available.
As stated previously there seems to be an issue with the `DataFrame` constructor when a 2d array is passed as index :

```python
>>> pd.DataFrame(data=[[1, 2], [5, 6]], index=[["a", "b"], ["c", "d"]])
     0  1
a c  1  2
b d  5  6
```
I think we could expect the same kind of creation between data and index, like :

```python
>>> pd.DataFrame(data=[[1, 2], [5, 6]], index=[["a", "b"], ["c", "d"]])
     0  1
a b  1  2
c d  5  6
```


Also, the current index from 2d array is created according to `pd.MultiIndex.from_arrays`.
But if we use a list of tuples as index, we obtain a single `Index` instead of a `MultiIndex` as we would obtain according to `pd.MultiIndex.from_tuples`.
```python
>>> pd.DataFrame([[1, 2], [5, 6]], index=[("a", "b"), ("c", "d")])
        0  1
(a, b)  1  2
(c, d)  5  6
```

I wonder if we should expect the following :

```python
>>> pd.DataFrame(data=[[1, 2], [5, 6]], index=[["a", "b"], ["c", "d"]])
     0  1
a b  1  2
c d  5  6
```

Before diving in code, I am interested to know if these two points are considered to be some issues.
Thanks !

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.
I opened #37670 to track the discussion re: DataFrame constructor & closing this. 

@LucasG0 we'd love to have you contribute other PRs of course! If you're interested in #37670 the way forward there is to get agreement in the issue on how to proceed
BUG: Index with right_index=True or left_index=True merge
State: closed

- [x] xref #28243 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Hi,

I would expect the output form the merge function as specified in #28243, i.e. if you join right with right_index=True the resulting DataFrame should have the right index.

The documentation says, that the index is passed on if it is part of the merge.

This change broke a few test, which expected in this case that the index was the index which was merged onto the base DataFrame. I fixed them temporarily until I know how to proceed in this case.

Should this as implemented be the desired behavior for the right_index=True case? 

The code is not final, it is just a first draft. I would have to change the tests for good after we decide how to implement this.

Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Stale

Comments:
> 
> 
> You'll also want to add a whatsnew note

I would like to wait until the solution is final. There are still a few open points in case of outer joins if my change should be the future behavior for left and right joins

I am not sure how to create the target index for an outer join whe the indexes are used in the join condition. My interpretation would be to merge them in case of equal names and create a MultiIndex in case of different names.
@phofl if you merge master will have a look
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.
@phofl if you can merge master will have a look
closing as stale. if you want to continue, pls ping and can re-open.
CLN: drop **kwds from pd.read_excel
State: closed

Drop ``**kwds`` from ``pd.read_excel``.

Wrt. "new" keyword parameter ``na_filter``: This parameter already exists in the doc string and is just masked in the ``kwds`` in the signature. If you follow the code paths all the way down to parsers.py, you can see that it indeed has a default of `True``.

Labels:
name: API Design
name: IO Excel
description: read_excel, to_excel

Comments:
Lgtm if green
thanks @topper-123
mask based multi-index assignment of column values described
State: closed

- [x] closes #34383
- [x] passes `black pandas`
not sure what this means, will look it up.
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


- ~~[ ] tests added / passed~~ 
this is an PR on documentation only, unit tests are not needed.
- ~~[ ] whatsnew entry~~
I think I don't need this as this is a PR related to documentation only.


Labels:
name: Docs
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Clean

Comments:
thanks @psteinb
CLN: Clean csv files in test data GH34427
State: closed

- [x] closes #34427
- [x] tests passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

I deleted the `iris.csv` and `tips.csv` files which are unused duplicates (i.e. those in `pandas/tests/io/data/csv`). 


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

Comments:
Thanks for the PR. Is it possible to do this the other way around? i.e. delete what's in tests/data and update the tests to make sure they point to tests/io/data/csv ?
yeah agree here, let's remove the test/data ones
@WillAyd & @jreback the reason why I did it that way is that a couple of files in the documentation reference the csvs in `tests/data`:
<img width="1440" alt="Screenshot 2020-05-28 at 16 27 37" src="https://user-images.githubusercontent.com/47032941/83285258-3d364580-a1de-11ea-8085-d14cd3313d24.png">

If you want to have the files in `tests/data` deleted & keep what's in `tests/io/data/csv` anyway, I can do that and change any references in the docs. 
@najann yes we'd like to fix this in tests / docs. 
@jreback there is also a `tips.csv` and `iris.data` file in doc/data... Should I clean that up & update any references (which exist only for `iris.data`), too?
I think OK to leave the doc ones alone for now
Actually made the above comment without realizing you already have to change some of the rst files; didn't realize they peered into the test folders like that

So I'm indifferent on that for now. I think this is good as is but can either fix all of the docs here or as a follow up.

I think test failure is unrelated; over to @jreback 
Re-running the failing travis job: https://travis-ci.org/github/pandas-dev/pandas/builds/692962783
Travis looks ok just hasn't updated status here
Thanks @najann very nice clean up and first PR
@MeeseeksDev backport to 1.0.x
Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

```
$ git checkout 1.0.x
$ git pull
```

2. Cherry pick the first parent branch of the this PR on top of the older branch:
```
$ git cherry-pick -m1 ef8567d0dc72ee54e21bbffe7b1c3f9ca5945909
```

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

```
$ git commit -am 'Backport PR #34458: CLN: Clean csv files in test data GH34427'
```

4. Push to a named branch :

```
git push YOURFORK 1.0.x:auto-backport-of-pr-34458-on-1.0.x
```

5. Create a PR against branch 1.0.x, I would have named this PR:

> "Backport PR #34458 on branch 1.0.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to [suggest an improvement](https://github.com/MeeseeksBox/MeeseeksDev).
[ENH] Allow pad, backfill and cumcount in groupby.transform
State: closed

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


Labels:
name: Groupby
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
Worth noting that the added aliases `pad` -> `ffill` and `backfill` -> `bfill` in `pandas/core/generic.py` are not necessary for resolving issue #31269.

However, the same thing was done in `pandas/core/groupby/groupby.py` so I thought it'd be nice to add these 2 aliases too. Besides, it makes the test `test_transform_transformation_func` simpler because of the added consistency.

Do let me know if you guys think this is inappropriate. I will remove them.

@WillAyd  btw should I mention the 2 new aliases in whatsnew? I don't have an issue number to tag it to though. Should I create an issue for that?
ping
Thanks @fujiaxiang another great PR - keep at it!
Fix MultiIndex .loc "Too Many Indexers" with None as return value
State: closed

- [x] closes #34318 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Three changes in indexing.py : 
- function _handle_lowerdim_multi_index_axis0 default return value was None, now it raises an Exception.
- function _getitem_lowerdim checks for a raised exception instead of a value None.
- function _getitem_nested_tuple checks for a raised exception instead of a value None(had to be changed because it also uses _handle_lowerdim_multi_index_axis0) 



Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex

Comments:
pls also add a whatsnew note, indexing bug fixes for 1.1
thanks @pedrooa
BUG: ensure_timedelta64ns overflows
State: closed

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


Labels:
name: Bug
name: Timedelta
description: Timedelta data type

Comments:
@WillAyd any thoughts on what to do here?  IIRC you implemented the C conversion functions for td64
So IIUC you are concerned about the value shown in the error message right? I did implement the time delta->ISO string methods which could use here, though not sure if necessary
updated to give a timedelta-specific exception message
updated with requested new exception class
updated+green
ping
CLN: liboffsets de-duplicate pickle code
State: closed

privatize/cdef functions that are now only used in liboffsets

Labels:
name: Frequency
description: DateOffsets

Comments:
@jbrockmendel if might be worth making another pickle file on 0.25.3 (and pushing to master) with all of the supported dateoffsets just to ensure we didn't break anything (unless they are already there). e.g. https://github.com/pandas-dev/pandas/blob/master/pandas/tests/io/generate_legacy_storage_files.py (though it looks like we most/all of these already)
TST #28981 comparison operation for interval dtypes
State: closed

As a complement of #28980, one test was modified to take into account this issue:
- [ x ] closes #28981
- [ 1 ] test modified
- [ 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
name: Interval
description: Interval data type

Comments:
thanks @OlivierLuG
TST #24444 converting Period to Timestamp
State: closed

I've tested intervals `ms`, `s` and `min`.
Note: the test raises an error for `ms` on v1.0.3, but is successful on master
Note: I did not choose to close this issue has the test fail for `ns` unit

[ ] #24444
[ 1 ] 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
name: Period
description: Period data type

Comments:
As per your comment and my last comment on #24444, I think a good tests could be like the ones below. However, no one is passing correctly.

```
@pytest.mark.parametrize("period_str, freq, expected",
        [
            ("2000-12-31 23:59:59.999999", "ms", 999999),
            ("2001-01-01 00:00:00.000001", "ms", 1),
        ],
    )
    def test_to_timestamp_precision_ms(self, period_str, freq, expected):
        # GH 24444
        result = Period(period_str).to_timestamp(freq=freq)
        assert result.microsecond == expected

@pytest.mark.parametrize( "period_str, freq, expected",
        [
            ("2000-12-31 23:59:59.999999999", "ns", 999999999),
            ("2001-01-01 00:00:00.000000001", "ns", 1),
        ],
    )
    def test_to_timestamp_precision_ns(self, period_str, freq, expected):
        # GH 24444
        result = Period(period_str).to_timestamp(freq=freq)
        assert result.nanosecond == expected
```
@OlivierLuG these are just tests. but is this issue actually fixed?
The issues are notre fixed. The tests all fail on master.
The flag `need test` mislead me. I've updated the main topic. Perhaps the flag `bug` is more appropriate.
Note: I tried to find a beginning point for this issue, but this is beyond my kwnoledge.
I've managed to resolve the microsecond issue + added test.
I've made a new PR to update master. Sorry, there iare conflict. I don't know how to handle that.
[x] closes #24444 
[x] `black pandas` passed
[x] news tests added
[x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Note, I'll will raise a new Issue. The `nanosecond` method returns `0` whatever I try.
TST: additional regression cases for slicing blockwise op (GH34421)
State: closed

Adding the other case from https://github.com/pandas-dev/pandas/issues/34367 to the test as well (although also fixed by the PR, it was failing with a different error originally, so might be worth it to add as test case as well).

cc @jbrockmendel  

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
thanks @jorisvandenbossche 
nice, thanks.  this has me wondering if we can use hypothesis to cover the space of block layouts
DOC: reduce API docs for offset aliases
State: closed

Test to see if this works with sphinx

Labels:
name: Docs
name: Frequency
description: DateOffsets

Comments:
Nice, there are some other aliases we could do the same for if this works.

Also some methods/properties that dont need to be advertised, e.g. `__call__`
Yeah, first need to see if it works ;) 
(locally it did, but I would actually expect to need to edit the BDay docstring to ensure it lists no methods/attributes, similar as eg the docstring of DatetimeArray)
It seems to work.

Can you say which are the aliases? Just the ones here?

https://github.com/pandas-dev/pandas/blob/5239cfa612b1798b8da46c7380c321c7e598214c/pandas/_libs/tslibs/offsets.pyx#L3487-L3494

I think thats all of them, yes.
Fix read_sql empty result with chunksize bug GH34411
State: closed

- [x ] closes #34411
- [ x] tests added / passed
- [ x] passes `black pandas`
- [ x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ x] whatsnew entry

`pd.read_sql` was returning an empty generator when chunksize is set and the query returns zero results. Now it correctly returns a generator with a single empty DataFrame (:issue:`34411`).

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

Comments:
@JohanKahrstrom looks like a merge conflict - can you fix that up? ping @jorisvandenbossche or @jreback for second review
Hello @JohanKahrstrom! 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 2021-01-14 17:55:48 UTC
@WillAyd, @jorisvandenbossche, @jreback, Merge conflict has been resolved, and commit history squashed.
... and fixed black formatting.
/azp run
<samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
Thanks Will, I was trying to figure out how to rerun the azure check...
can u merge master and we will look again
@JohanKahrstrom can you add a release note
@jreback @simonjayhawkins Added release note, merged with master. Unrelated checks are failing, not sure what to do about those...
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.
@JohanKahrstrom Can you merge master to fix conflicts? Looks like this might be close to ready?
/azp run
<samp>
Commenter does not have sufficient privileges for PR 34429 in repo pandas-dev/pandas<br>

</samp>
@dsaxton I've merged with master. There's 3 failing tests unrelated to my code change, and the Azure test suite failed again (I don't have sufficient privileges to re-run them).

@jreback are you happy with my argument for not implementing your suggested change (as far as I can see it won't work)?
@JohanKahrstrom can you merge master once more? (There's a conflict in whatsnew)
@arw2019 Done.
This looks ready to go.

cc @jreback @dsaxton 
thanks @JohanKahrstrom