BrandonFors/pandas-github-issues huggingface dataset:
f string in io.common
State: closed

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


Labels:
name: Code Style
description: Code style, linting, code_checks
name: Clean

Comments:
Thanks for the PR! Please run black on your changes - see the code_check github actions
```
Checking black formatting
would reformat /home/runner/work/pandas/pandas/pandas/io/common.py
Oh no! 💥 💔 💥
1 file would be reformatted, 875 files would be left unchanged.
```
If you merge master as @WillAyd suggested above test failures should be fixed 
I think CI failure is unrelated yet again - can you merge master one more time to be sure?
Hi,
I've attempted to merge to master, however, receive the following error messages. Any help much appreciated!

![errormsg](https://user-images.githubusercontent.com/21073989/70384297-94dfac80-1931-11ea-8f2d-313156fea5d0.JPG)

@daniellebrown you need to ``git push origin shiny-new-feature -f``
@daniellebrown might want to try one more of:

```
git fetch upstream
git merge upstream/master
git push origin shiny-new-feature
```

To resolve CI failures
@daniellebrown https://github.com/pandas-dev/pandas/pull/30022#issuecomment-561434493 see https://dev.pandas.io/docs/development/contributing.html#python-pep8-black
Any advice on how to fix this error: 

would reformat /home/runner/work/pandas/pandas/pandas/io/common.py
Oh no! 💥 💔 💥
1 file would be reformatted, 880 files would be left unchanged.

I've run black several times and it doesn't seem to do the trick. 
thanks @daniellebrown  very nice
thanks @jreback @WillAyd @simonjayhawkins !
Replaced !r with repr()
State: closed

- [x] reference #29886
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Issue: https://github.com/pandas-dev/pandas/issues/29886

@WillAyd 

Labels:
name: Code Style
description: Code style, linting, code_checks
name: Clean

Comments:
Thanks for the PR. Looks good and I think CI failures are unrelated / have been fixed on master since this was created.

To be sure, can you locally on your `fstring-sorting` branch run:

```sh
git fetch upstream
git merge upstream/master
git push origin fstring-sorting
```

I think should get CI green for this. Flag me down at PyData if you have any questions
Looks like a merge conflict @kvn219 can you fix that up and re-push?
> Looks like a merge conflict @kvn219 can you fix that up and re-push?


Hey @WillAyd, I'm not sure what to do. I'll go to HackLA next week and ask for help.
Thanks @kvn219
CLN: no need to broadcast axes for ndim<=2 (pytables)
State: closed

It takes a little bit of work to show, but `self.index_axes` is always length-1 on L4243, which means that nindexes == 1 (the assertion added in the code).

This ensures that the condition `if i > 0 and i < nindexes` on L4239 is is never satisfied, so `bindexes` is unnecessary.

Labels:
name: Clean

Comments:
thanks
CLN: pytables _set_tz/_get_tz
State: closed

removes preserve_UTC argument, which never did anything, and didnt have a clear purpose

Labels:
name: IO HDF5
description: read_hdf, HDFStore
name: Clean

Comments:
updated per comments

BTW thanks @jreback and @WillAyd for putting up with the deluge of PRs in this file.  I think once the statefulness issues are addressed it will become much more maintainable long-term
ok sure
DOC: Added "Raises `KeyError`"
State: closed

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


Labels:
name: Docs

Comments:
> Not sure about these changes - is this purely a stylistic thing you are aiming for?

Yes, indeed purely stylistic change, I'll revert.
thanks @MomIsBestFriend
DOC: Fix to lzma.LZMAFile
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: Docs
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
It would need to be Type[lzma.LZMAFile], though not sure if we can do that given this is technically an optional dependency from the stdlib
Does Type[IO] work? May be the best bet 
LZMA has been part of the standard library since python 3.3. We should just assume it’s installed, that will simplify this a bit.
> LZMA has been part of the standard library since python 3.3. We should just remove assume it’s installed, that will simplify this a bit.

I'm pretty sure that just recently i had to do an apt-get install to get this working on an ubuntu 18 machine
We had an issue previously with that assumption, hence the existence of this compat to begin with. See #27575
Ok, that’s surprising. Yeah, changing it isn’t possible then, I guess...
> It would need to be Type[lzma.LZMAFile], though not sure if we can do that given this is technically an optional dependency from the stdlib

```mypy```'s log:

```
Success: no issues found in 808 source files
```

> Does Type[IO] work? May be the best bet

```mypy```'s log:

```
pandas/io/common.py:468: error: Too many arguments for "IO"
Found 1 error in 1 file (checked 808 source files)
```

> > It would need to be Type[lzma.LZMAFile], though not sure if we can do that given this is technically an optional dependency from the stdlib
> 
> `mypy`'s log:
> 
> ```
> Success: no issues found in 808 source files
> ```
> 
> > Does Type[IO] work? May be the best bet
> 
> `mypy`'s log:
> 
> ```
> pandas/io/common.py:468: error: Too many arguments for "IO"
> Found 1 error in 1 file (checked 808 source files)
> ```

I just found a bug (I think), unless I'm missing something obvious.

When  ``` _get_lzma_file``` is annotated with ```-> Type[IO]```, normally ```mypy``` will say:
```
pandas/io/common.py:468: error: Too many arguments for "IO"
Found 1 error in 1 file (checked 808 source files)
```

But if the ```lzma``` module is imported at the beginning of the file you ```mypy``` will say:
```
Success: no issues found in 808 source files
```

---

So two questions actually:
* Is this a bug?
* What option do we prefer? (if any)
Thanks @MomIsBestFriend

Can come back to types later for the whole module if something you are interested in
CI: troubleshoot openpyxl failures
State: closed

tested on OSX 36 and 37, we'll see if that generalizes

Labels:
name: CI
description: Continuous Integration

Comments:
@jorisvandenbossche I think noticed this last week
> @jorisvandenbossche I think noticed this last week

Right [here](https://github.com/pandas-dev/pandas/pull/29862#issuecomment-559013181) (comment).
is there a better workaround someone has in mind to get the CI back to green?
@TomAugspurger gentle ping
thanks @jbrockmendel happy to keep Ci green
keep an eye open for when can revert
Might be awhile according to openpyxl devs:

https://bitbucket.org/openpyxl/openpyxl/issues/1373/broken-writer-with-lxml-defusedxml
CLN: test code no longer shared with sparse
State: closed

broken off from #29985.

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

Comments:
great!
this PR is showing up as merged but still open, huh?
oh that's weird
https://github.com/pandas-dev/pandas/commit/c5f2084160f765f6b7495b987a198f9db5e972cd definitly merged

lol i clicked the button :->
DEPR: plotting deprecations
State: closed

Input/help needed on two outstanding questions:

1) Do we just remove `pandas.tseries.converter` after this?
2) Some of these deprecations are old enough that I couldn't track them down via the blame, so they currently have "(:issue:`????`)" in the whatsnew.

Labels:
name: Visualization
description: plotting
name: Deprecate
description: Functionality to remove in pandas

Comments:
updated+green
thanks @jbrockmendel
changed "fun !r" -> "repr(fun)"
State: closed

As described in the following issue
, usage of !r is currently redundant and so changing to f strings in place of it.

- [x] ref https://github.com/pandas-dev/pandas/issues/29886
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks
name: Clean

Comments:
Hello @souravs17031999! 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:





* In the file [`pandas/core/strings.py`](https://github.com/pandas-dev/pandas/blob/fc5135ef32e3118dd90cf783574ee792939d38e8/pandas/core/strings.py):

> [Line 1938:1](https://github.com/pandas-dev/pandas/blob/fc5135ef32e3118dd90cf783574ee792939d38e8/pandas/core/strings.py#L1938): [W293](https://duckduckgo.com/?q=pep8%20W293) blank line contains whitespace


I will open new PR after going through it again.
TYP: some (mainly primative) types for core.generic
State: closed

mypy 0.750 has an experimental feature for static inference of annotations see https://mypy.readthedocs.io/en/latest/mypy_daemon.html#static-inference-of-annotations

have tried it out on functions in core.generic. The annotations add here are mainly the ones that matched the docstrings.


Labels:
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
Question about bool vs bool_t consistency, not a blocker, otherwise LGTM
thanks @simonjayhawkins happy to address the bool/bool_t issue later; we should likely just standardize on bool_t no?
DEPR: remove get_values, SparseArray.values
State: closed

The cleanups in libreduction are made possible by SparseArray.values going away.

Labels:
name: Sparse
description: Sparse Data Type
name: Deprecate
description: Functionality to remove in pandas
name: Clean

Comments:
I would have thought these changes would have been fine...just need to diagnose the CI failures.
I think there is some JSON code that can get cleaned up as part of this:

https://github.com/pandas-dev/pandas/blob/ed20822a54e3863b393554ecca801654af105555/pandas/_libs/src/ujson/python/objToJSON.c#L216

> I think there is some JSON code that can get cleaned up as part of this:

Nice.  Want me to do that here or save for follow-up?
Up to you
> > I think there is some JSON code that can get cleaned up as part of this:
>
> Nice. Want me to do that here or save for follow-up?

Let's do that here I would say, it's from the same PR that deprecated SparseArray.values: https://github.com/pandas-dev/pandas/pull/26421 (you are already removing part of the things of that PR that can be removed)
removed examples from docstring, left the json since it wasnt obvious what to do in the places where is_sparse_array is currently called
>  left the json since it wasnt obvious what to do in the places where is_sparse_array is currently called

From looking at my old PR, I think you can just remove the `is_sparse_array` where it is used (so change it back to how it was before that PR). SparseArray won't have a `values` attribute anymore, so that check will work fine now (and not trigger a warning).

removed is_sparse_array, green
Thanks @jbrockmendel
STY: F-strings
State: closed

- [x] ref https://github.com/pandas-dev/pandas/issues/29547
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Strings
description: String extension data type and string data
name: Code Style
description: Code style, linting, code_checks

Comments:
(Unrelated) Test failure notwithstanding, these changes look good
thanks @MomIsBestFriend
DEPR: remove real, imag, put, and str.partition kwarg
State: closed


Labels:
name: Deprecate
description: Functionality to remove in pandas
name: Clean

Comments:
These changes I thought would have been fine.

Seem to have instead triggered multiple test failures...
Looks like ptp behavior is fickle depending on numpy version, will revert that and save it for a later day
thanks @jbrockmendel 

small followup indicated by @jorisvandenbossche  above
CLN: assorted (non-pytables) cleanups
State: closed

includes a couple of whatsnew notes that fell through the cracks

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff
name: Clean

Comments:
@simonjayhawkins mypy is complaining about instantiating a class where we use abc.ABCMeta, thoughts?
@gfyoung broke pieces off into #30006, #30007, will revert those from this PR
> @simonjayhawkins mypy is complaining about instantiating a class where we use abc.ABCMeta, thoughts?

try this..

```diff
diff --git a/pandas/core/computation/engines.py b/pandas/core/computation/engines.py
index a4eaa897c..9b813bb4c 100644
--- a/pandas/core/computation/engines.py
+++ b/pandas/core/computation/engines.py
@@ -3,6 +3,7 @@ Engine classes for :func:`~pandas.eval`
 """
 
 import abc
+from typing import Dict, Type
 
 from pandas.core.computation.align import align_terms, reconstruct_object
 from pandas.core.computation.ops import _mathops, _reductions
@@ -139,4 +140,7 @@ class PythonEngine(AbstractEngine):
         pass
 
 
-_engines = {"numexpr": NumExprEngine, "python": PythonEngine}
+_engines: Dict[str, Type[AbstractEngine]] = {
+    "numexpr": NumExprEngine,
+    "python": PythonEngine,
+}
```
can you rebase
thanks @jbrockmendel if you can address @gfyoung comment in a followup
STY: F-strings
State: closed

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


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
How come the failed test is related to any of the changed files?
REF: move attribute setting towards the constructor
State: closed

The end goal is to minimize the amount of attribute setting that is done outside of the constructor (or create_for_block which is a classmethod and ill give it a pass).

create_for_block is called in one place, and immediately after that set_atom is called on the result.  set_atom does a ton of attribute setting, so the idea here is to gradually move that attribute setting up into create_for_block.

Related but kept for a separate branch, most of the affected methods have `block` arguments, these can all be changed to operate on block.values, which will narrow the exposure to internals.

Labels:
name: IO HDF5
description: read_hdf, HDFStore
name: Clean

Comments:
sure
CLN: explicit kwargs for select
State: closed

Questions for @jreback on follow-ups:

1) Note in `read_hdf` kwargs is still present in the signature, but i put an assertion that they are empty.  In principle we could pass more junk to HDFStore, but we never do in tests.  Should we allow it, or can those kwargs be deleted altogether?

2) the current signature for `select_column` is `(self, key: str, column: str, **kwargs)`.  These kwargs are passed to `read_column`, which would accept "where", "start", "stop".  In tests we pass start and stop, but never where.  Can we rule out "where"?


Labels:
name: IO HDF5
description: read_hdf, HDFStore

Comments:
> Questions for @jreback on follow-ups:
> 
> 1. Note in `read_hdf` kwargs is still present in the signature, but i put an assertion that they are empty.  In principle we could pass more junk to HDFStore, but we never do in tests.  Should we allow it, or can those kwargs be deleted altogether?
> 2. the current signature for `select_column` is `(self, key: str, column: str, **kwargs)`.  These kwargs are passed to `read_column`, which would accept "where", "start", "stop".  In tests we pass start and stop, but never where.  Can we rule out "where"?

IIRC I answered 1) elsewhere. we pass on kwargs in the constructor to ``.open_file``; these are keywords that are used in the construction deep in PyTables for how to open the backends, e.g.https://www.pytables.org/cookbook/inmemory_hdf5_files.html, e.g. you could pass ``driver=`` and it would work.

2) yes I think you can remove ``where``
suggestion for a way out to remove the kwargs arg in read_hdf & HDFStore
CLN: explicit kwargs and docstring for create_index
State: closed

Side-note, in `create_index`, none of our tests hit the `v.remove_index()` calls.  If we could rule those out, this function could be simplified quite a bit.

Labels:
name: IO HDF5
description: read_hdf, HDFStore

Comments:
thanks.

``.remove_index()`` was added for to have the mirror routine, but yes likely not used.
API: Uses pd.NA in IntegerArray
State: closed

Just a WIP for now. I haven't closely reviewed this since pd.NA was merged into master.

cc @jorisvandenbossche 

Closes https://github.com/pandas-dev/pandas/issues/15375

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: API Design
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Still some failing tests. Will have to put this on hold for a day or two now.
The newly added tests from the other PR are failing (they still assume bool numpy output)
```
pandas/tests/test_register_accessor.py:39: error: Module has no attribute "register_series_accessor"
pandas/tests/test_register_accessor.py:40: error: Module has no attribute "register_dataframe_accessor"
pandas/tests/test_register_accessor.py:41: error: Module has no attribute "register_index_accessor"
```

I'm not able to reproduce these locally.
What's the status of this? This is ready to go? (with the special cases for pow etc already merged in other PRs)
> What's the status of this? This is ready to go? (with the special cases for pow etc already merged in other PRs)

I think it's ready. All the `__pow__` stuff was handled elsewhere. The only pow-related changes were to add tests with `pd.NA` like https://github.com/pandas-dev/pandas/pull/29964/files#diff-3523d268da9f1c7d4c82d4d27241ed07R373.
I am planning to merge this in a bit. I am not sure if @jreback's review was a full one, but the comments have been addressed/answered. But it would be good to have this in master for IntegerArray as well, and this is blocking some follow-ups (completing `to_numpy` in #30322, creating the base class). There can be more review in those PRs I think.
let me have a look a n hour or 2
@jreback did you still want to / have time to take a look?
let me look
Thanks, added whatsnew for those changes.
All green. Anything else @jreback or were the two in https://github.com/pandas-dev/pandas/pull/29964#pullrequestreview-337098618 it?
thanks @TomAugspurger
STY: "{foo!r}" -> "{repr(foo)}" #batch-5
State: closed

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


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
Please merge master
lgtm ping on green
ping @jreback 
thanks @MomIsBestFriend
STY: "{foo!r}" -> "{repr(foo)}" #batch-4
State: closed

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


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
thanks @MomIsBestFriend
Make kwargs explicit in put, append
State: closed

Made possible by #29951, progress towards #20903 

@jreback input needed on whether certain not-covered-in-tests cases should be considered possible.  See that both `put` and `append` still contain `**kwargs`.  In all extant tests, kwargs is always empty.  But there are remaining keywords that write_to_group (which both put and append call) accepts that are not in the put/append signatures.

So the question is: is there a reason why put/append would accept different subsets of the writing keywords?  If not, they should both have the same explicit kwargs as write_to_group.

Once this is pinned down, we can get explicit all the way up the stack to NDFrame.to_hdf

Labels:
name: IO HDF5
description: read_hdf, HDFStore

Comments:
> Made possible by #29951, progress towards #20903
> 
> @jreback input needed on whether certain not-covered-in-tests cases should be considered possible. See that both `put` and `append` still contain `**kwargs`. In all extant tests, kwargs is always empty. But there are remaining keywords that write_to_group (which both put and append call) accepts that are not in the put/append signatures.
> 
> So the question is: is there a reason why put/append would accept different subsets of the writing keywords? If not, they should both have the same explicit kwargs as write_to_group.
> 
> Once this is pinned down, we can get explicit all the way up the stack to NDFrame.to_hdf

these are likely ok. The reason for extra keywords is mainly on opening of the store itself, where PyTables passes thru keywords (e.g. to specify a different kind of backend: https://github.com/PyTables/PyTables/blob/master/tables/file.py#L219)
> these are likely ok. The reason for extra keywords is mainly on opening of the store itself,

To be clear, I'm referring to keywords that we have established are accepted by _write_to_group, i.e. not ones being passed to HDFStore.
> To be clear, I'm referring to keywords that we have established are accepted by _write_to_group, i.e. not ones being passed to HDFStore.

then i think ok to remove
Updated:

- updated annotations
- removed kwargs
- to do this, had to enforce nan_rep cannot be passed with append=True (double check this is desired)?
- removed no-longer-allowed dropna, fletcher32 kwargs from NDFrame.to_hdf docstring (double check this is desired)?
updated+green (ex an unrelated jinja2 problem ill work on in the morning)
thanks
DOC : Typo fix in userguide/Styling
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Fixed a typo in the user-guide/Styling: https://pandas-docs.github.io/pandas-docs-travis/user_guide/style.html#Sharing-Styles

In the line:
`Notice that you’re able share the styles even though they’re data aware. The styles are re-evaluated on the new DataFrame they’ve been used upon.`

Labels:
name: Docs

Comments:
thanks @koushikgk
DEPR: Change default value for CategoricalDtype.ordered from None to False
State: closed

- [X] closes #27203
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

A quick summary of the deprecation details can be found here: https://github.com/pandas-dev/pandas/pull/26403#issuecomment-507351745

Labels:
name: Categorical
description: Categorical Data Type
name: Deprecate
description: Functionality to remove in pandas
name: Clean

Comments:
> I don't fully remember the previous discussion (should look back at it), but do we want to allow ordered=None in the main constructor?

I had forgotten too but apparently my past self anticipated this and wrote a nice summary here: https://github.com/pandas-dev/pandas/pull/26403#issuecomment-507351745.  Also edited my first comment so this link in included at the top of the PR.  Looks like the decision was to maintain the `ordered=None` behavior.

> If we keep it in the public constructor, we should probably document it?

I can add some documentation for this in a bit (after sleep + work) if we want to advertise this behavior.  There were previously some concerns that this might not be behavior we want to encourage (xref https://github.com/pandas-dev/pandas/pull/26403#discussion_r299160647) since this is in part an implementation detail to handle `"category"`, and the same behavior can be achieved by explicitly specifying `ordered`.
@jschendel needs rebase
merged master and added a note regarding the usage of `ordered=None`
@jschendel Thanks for the update and explanation !