minor inconsistency in Categorical.remove_categories error message
State: closed

This pull request fixes minor inconsistency in Categorical.remove_categories error message

- [x] closes #28669 
- [ ] tests added / passed
-  passes `black pandas`
-  passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
-  whatsnew entry

Changed the error message to show invalid removals as a set. Added tests for removal of null from the categories. Parameterized pytest. 

Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Categorical
description: Categorical Data Type

Comments:
Hello @punndcoder28! 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 2019-10-09 17:53:34 UTC
Nitpick: in the future please give the PR an informative title.  Most reviewers won't know off the top of their heads what 28677 is about.
Yes my bad @jbrockmendel. I realized it after opening the PR sorry.
I had tests as it were suggested by @simonjayhawkins. But there were errors and I didn't know how to handle that. If I could get some guidance I will surely add those and commit the changes.
If you could tell me what tests I should create I'll create those and commit @jreback.
@punndcoder28  suitable tests would be similar to those in pandas/tests/arrays/categorical/test_api.py in  https://github.com/pandas-dev/pandas/pull/28339/commits/8298620a3a2e2c88b922b5da5b76609e62c5d852
`assert [NaN, b, c, NaN]\nCategories (2, object): [b < c] is None`. This shouldn't cause an error right. As  `[b < c] is None` states the statement `assert res is None` should evaluate to True and should not raise an `AssertionError`. Am I understanding this the wrong way?
Hey @simonjayhawkins. I've made the changes as requested. Could you check and let me know if any further changes are needed?
@punndcoder28 Thanks for the updates. can you also add a what's new.
Hey @simonjayhawkins. I've made the requested changes can you check it out and let me know if you need any more changes.
> @punndcoder28 generally looks good. a couple of small changes. also need to merge master to resolve merge conflict. ping on green.

How do I resolve a conflict? I tried doing it manually but didn't work.
> How do I resolve a conflict? I tried doing it manually but didn't work.

see https://dev.pandas.io/docs/development/contributing.html#updating-your-pull-request
```
git fetch upstream
git merge upstream/master
```
I did what was mentioned here 

> ci is failing, see https://dev.pandas.io/docs/development/contributing.html#python-pep8-black

But still the tests are failing.
You need to run `isort pandas/tests/arrays/categorical/test_api.py`
Hmm is this actually changing anything on master? I ran one of the test cases but already see the intended result?

```python
>>> pd.Categorical(["a", "b", "a"]).remove_categories(["c", "c"])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/williamayd/clones/pandas/pandas/core/arrays/categorical.py", line 1132, in remove_categories
    raise ValueError(msg.format(not_included=not_included))
ValueError: removals must all be in old categories: {'c'}
```
it's changeing the output of `cat = cat.remove_categories(["c", np.nan])`

from `ValueError: removals must all be in old categories: ['c']`

to `ValueError: removals must all be in old categories: {'c'}`

for all other cases the message remain unchanged and tests are added for these. i.e.

`cat = cat.remove_categories(["c"])`
`cat = cat.remove_categories("c")`
`cat = cat.remove_categories(["c", "c"])`
Thanks @punndcoder28