BrandonFors/pandas-github-issues huggingface dataset:
date_range should (at least optionally) deal with right-open intervals
State: closed

It is confusing that, albeit the name suggests otherwise, `date_range` behaves differently compared to the standard Python `range` generator in that it returns the right-closed interval.


Labels:
name: Enhancement
name: Datetime
description: Datetime data dtype

Comments:
@filmor you can submit a PR if you would like, but this is essentially label based indexing, so the python convention doesn't apply here. That side, having a `closed=left|right'` would not be a bad option

I still have to build some test-cases for the new inputs before this is ready for merge.

I think `closed` can only apply to the right side (too confusing othewise). Also pls add a release notes and v0.13 entry (enhancements)

The current commit contains a release note, but I don't understand your comment on the parameter. What is wrong with being able to have open, left-open, right-open and closed intervals? There are uses for all of them and the interface seems pretty obvious to me …

take a look at `tseries/resample.py/TimeGroup`

`closed=None` is the default, can be 'right' or 'left'....let's go with this so the API is not changed

`None` is your equivalent of `True`, `False` (meaning both open) is not supported

can't change the API ATM different from what exists in the rest of pandas.
(you are welcome to do that in 0.14, but that would require some more discussion)

@filmor need some tests for this; e.g. with closed taking on each value and that the generated range is correct (for several freqs should be good).  should go in `tseries/tests/test_daterange.py`

Could you have a look at the Travis output? An error occurs in a place, where I didn't change anything(tm), for some reason the `Timestamp`s in `DatetimeIndex._generate` are converted back to `datetime.datetime` without a `tz` attribute, leading to an error. However this only seems to happen after the `start += offset` or `end -= offset` call ...

@filmor did you fix the bug?

@filmor how's this coming?

thanks for the PR!
DOC: str.match should (obviously) be str.extract.
State: closed


Labels:

Comments:
@danielballan can u stick an example in the actual doc string of extract if I have a chance (so when u look at the doc thru the API it shows up), eg http://pandas.pydata.org/pandas-docs/dev/generated/pandas.core.strings.StringMethods.extract.html?highlight=extract#pandas.core.strings.StringMethods.extract

OK, will try to get to that this week.
TST: Groupby filter tests involved len, closing #4447
State: closed

closed #4447 (tests only, so no release notes necessary)

...which was apparently fixed by #4657. More tests looking for shape-related bugs in filter would be nice. So far I have no discovered any.


Labels:

Comments:
maybe add some more functions here? that are mostly errors

e.g. return a weird shape, return different dtypes, raise an error 

```
def f(x):
     if len(x) > 1:
           raise TypeError():
     return True

def f(x):
      return np.nan

def f(x):
       return x

def f(x):
       return x == 1
```

Thanks for those examples. In this WIP commit, the commented-out tests are not raising as they should, so some work is needed on filter itself. Also, there is some inconsistency between ValueError/TypeError depending on DataFrame/Series. Will keeping working...

yep....assume the user doesn't know what they are doing and don't read the docs :) then try to do something reasonable (including raising a helpful message that filter must return a boolean value!)

@danielballan ready to go on this?

Nearly. First, a question: if filter returns a non-boolean value, should we proceed to evaluate its truth? (As far as I understand, this calls `__nonzero__` in 2.x, `__bool__` in 3.x.)  Specifcally, should `grouped.filter(lambda x: 1)` raise or behave like `grouped.filter(lambda x: True)`? What about `np.nan`, which also evaluates `True`? I lean toward raising on `np.nan` but tolerating `1`. Opinions?

I'd expect it to accept anything bool-like, potentially with special case for nan being equivalent to False (and producing ndarrays with filter will raise as expected)

OK, assuming Travis does not object, this is good to go.

looks good

at some future point an example for docs/cookbook which used a non-trivial filter

@jtratner @cpcloud comments?

@danielballan pls squash this down when you are ready

Ready, pending discussion on & vs. and with cpcloud above.

this looks fine....@cpcloud

this is ok...merging
TST: Add skip test to excelwriter contextmanager
State: closed

Fixes #5094.


Labels:

Comments:
Side note - reverting #5058 not because of what version should be xlrd base, but rather because multiple places need to be changed to test with older version of xlrd and we should just go with one min version for xlrd.

And here's where it was decided to require 0.9 - #1629 . Apparently had to do with Python 3 support?
DOC: CONTRIBUTING.md: Gold plating: syntax, punctuation, Markdown format...
State: closed

...ting

:sparkles:


Labels:

Comments:
... I don't know how many times I've wanted to deep link to the EXCELLENT commit message tags section, which doesn't have its very own heading.

can you squash?

I have comments on this.

@jreback 

> can you squash?

I've been in Mercurial land, working with MQ. How do I squash?

@jtratner 

> I have comments on this.

Shoot.

To squash:

``` sh
# add the remote if you haven't already
git remote add upstream git://github.com/pydata/pandas.git
git fetch upstream
git rebase upstream/master # shouldn't be any merge conflicts
git rebase --interactive upstream/master  # this is where you squash
```

@cpcloud 
Thanks.

@jreback 
Smushed, I think.

When you squash there should be fewer commits than before (you're essentially combining commits). You've now got some commit history from recent master _and_ repeat commits that you'll need to clean up. Here's how:

**This will destroy any changes you made in between your previous commits and your squash attempt**. You can recover them, but lets only deal with that if we have to.

``` sh
git reset --hard HEAD~6  # go back to your original before the squash
```

@westurner sorry for holding this up, looks good and thanks. Please make that one change and I'll merge it.

@jtratner 
Done and done.

Looks like you've still got a bunch of extra history. Can you clear that up? We can merge as soon as you do that. Thanks!

@westurner Why did you close?

@cpcloud 
I guess this was not a good sequence of commands for folding patches:

```
git reset --soft HEAD~7
git commit -m "..."
git push origin :patch-2
```

... merge conflicts

A soft reset will simply put the commits from `HEAD` to `HEAD~7` (inclusive) in the index, as if you had made all of the changes at once and had just staged them. It won't correct any merge conflicts or other issues

Happy to help u some more with `git`. I'm very patient.

https://travis-ci.org/westurner/pandas/jobs/12120689:

```
Using worker: worker-linux-3-2.bb.travis-ci.org:travis-linux-6

$ export NOSE_ARGS="not slow"
$ export FULL_DEPS=true
$ export GUI=qt4
$ git clone --depth=50 --branch=patch-2 git://github.com/westurner/pandas.git westurner/pandas
Cloning into 'westurner/pandas'...
fatal: unable to connect to github.com:
github.com: Temporary failure in name resolution

[...]
```

that's a random failure on travis's end (or maybe github's)

Thanks. How do I avoid that extra commit in the PR? I think I `reset HEAD~1` one too many times.

(Tornado-y weather on the ground here in Omaha at the moment)

Do these commands exactly as I have them here

``` sh
git reset HEAD~1
git stash
git reset --hard upstream/master
git stash pop
git add .
git commit -m 'WRITE AN AWESOME MESSAGE HERE'
git push --force
```

but write a differernt commit message

bravo! hope that helped and that u understand git a bit more than before :)

Thanks so much!

@jtratner 
Should be good to merge.

thanks!
CLN: Remove internal classes from MultiIndex pickle.
State: closed

FrozenNDArray had made it into the MI pickle. No reason to do that and just
complicates pickle compat going forward. Now they just output ndarray
instead (which also avoids the unnecessary nested pickle previously
occurring).

Fixes #5076.


Labels:

Comments:
perfect..so this will still exist, just not in pickles

@jreback so something like this, where it's fixing something that never made it to any release. Should that go in release notes?

nope (you could annotate the original) or even remove it
BUG: Give correct ndim, shape and size properties
State: closed

Treats it like 2d array rather than array of tuples. I like it because
it's more informative - but we could also just describe `.values`.

Fixes #4842.

Still working out some kinks.


Labels:

Comments:
Woah, this causes a segmentation fault? It must be because you can't override those properties without messing up numpy.

I'm going to push this to 0.14 when MI won't be a subclass of ndarray.
BUG: Make Index, Int64Index and MI repr evalable
State: closed

MI repr was missing a comma between its arguments and Index reprs needed
to quote their dtypes.

Only talking about Index, Int64Index and
MultiIndex. Tseries indices (like PeriodIndex and DatetimeIndex) are
more complicated and could be covered separately.


Labels:

Comments:
To be clear, the major change is to add commas between MI attributes and quote dtypes for Index. So, pre this PR:

``` python
repr(Index(range(10)))
Int64Index([1, 2, 3, 4, 5, 6, 7, 8, 9, 10], dtype=int64)
```

With this PR

``` python
repr(Index(range(10)))
Int64Index([1, 2, 3, 4, 5, 6, 7, 8, 9, 10], dtype='int64')
```

Everybody okay with that?

ok with this

Okay, I'll make sure it still passes and then push.
API: default export for to_clipboard is now csv/tsv suitable for excel (GH3368)
State: closed

closes #3368


Labels:

Comments:
We should have a setting `io.clipboard.sep` . Can we change to `\t` for windows/osx by default and I guess `','` for Linux? (set the defaults in config_init, right?)

`to_clipboard()`'s current behaviour is useful for other things, how about `df.to_clipboard(excel=False)`?

@jreback `excel=False` is fine, but that'd just be a synonym for `sep=','` right?

I think we make \t the default, maybe make it a config option as well? `io.clip.sep`?

@y-p I will update docs/announcemnt not sure an explicity excel kw is necessary

`df.to_clip()` can be used to paste a nicely-formatted textual table into a document, that's useful as well.
Not sure making an arg for method invocation into a config option makes sense.

@y-p I actually need to use a sep of ',' to paste as I copy from a linux X window to excel; prob not typical

there is no easy way to actually form the object prior to writing to the clipboad; eg. we are using to_csv and not to_string.

right. I thought it'd be best if `to_clip(excel=False)` kept the current behaviour, and `to_clip(1)`
generated a csv. 

@jtratner @y-p further comments?  do we need an `excel` kw?

What a bummer to be ignored like that. _sadness_.

@y-p what do you mean? I never saw a comment when I posted a request for further comments...

Isn't it clearly better to extend the existing functionality rather then replace it? I made 
that point 3 times early on.
It's totally fine you don't agree, but, what's the reasoning?

this doesn't actually change much, except for replacing space separation with tabs sep, which _appears_ to be more compatible with various systems. I believe you pointed out the issue, which tab sep fixes, or is this not the case?

tabs allow you to paste into excel, yeah, but now you can't get an aligned textual table into the clipboard.
`sep=' '` doesn't replicate the old behavior, try it. 

and how would you propose for an API to handle both of these somewhat discordant cases?

as IMHO I would think pasting to excel is much more important

What's wrong with what I suggested above?:

``` python
def to_clipboard(excel=False):
    if not excel:
       to_string() # -> clip, original behavior
    else:
       to_csv(sep='\t') # -> clip , new behavior

```

Also, why decide which is more important when it's so easy to allow both while
avoiding breakage to existing behaviour at the same time?

ok....that's reasonable....guess I misunderstood what you meant....would have have a problem with default `excel=True`? it breaks compat, but I think is more intuitive

I guess there's not much harm in changing the default,since it's easy
to request the old behavior. It's for interactive use only anyway. so yeah, should be fine.

+1 on excel=True as the default, if for no other reason that it'd be a
whizbang feature to show off.  Also, I've spent most of the day
copy/pasting into Excel so that's something.

lol!

Thanks jeff, it's _just right_ now. :)

:)
Issue 1014 Possibly attempt object -> float64 coercion in to_sparse
State: closed

closes #1014

I've added a check for dtype=object and try to coerce it to a float in frame.to_sparse


Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Sparse
description: Sparse Data Type

Comments:
hey interesting idea here, can you fix all the issues that I pointed out and then ping us when they're fixed so we can discuss?

@jdowns why is your data object (that are really floats) in the first place? that actually is somewhat hard to do, unless you are casting it. or is the intention to filter out non-numerics first?

@jreback this was the problem from issue #1014.  The person who opened the ticket had that problem.  I'm not sure how it came up.

I see that now...thanks...

ok...glad you have an interest in to_sparse! FYI it changed quite a lot in 0.13. So in theory, you can support multiple dtypes in the sparse objects, so this might not be strictly necesary. And as I said above its hard to actually have non-object numeric data.

This is probably accomplished by just a `self.convert_objects(convert_numeric='force')` to force conversion (after you have detected an object-dtype, or possible, `self._get_numeric_data()`. Really depends on what the purpose is.

@jreback I did have the issue of getting objects when I expected floats once.  I was reading a fixed width data file that had dashes to make it look better to the human eye.  It was easy to skip that line, but I could see somebody inadvertently getting a data frame like that and just wanting to go on with their computation. Judging by the example in issue #1014 I think the use case is similar. 

@jreback the problem is that you'd need to do convert_objects and then cast to float, right?

@jreback Yes, that's a solution to the problem I encountered. 

`convert_objects(convert_numeric='coerce')` will do all of this; no casting necessary it will cast as needed and turn non-numeric into nan (if you pass force), works on series and frames. astyping is just for changing types (e.g. float64 -> float32), it _will_ in some cases do the same thing but not generally

What if it returns int instead?

it will attempt downcasting as needed (e.g. there's a defined conversion com._possibly_downcast_to_dtype), in essence if its a float and compares equal to a casted version and no nulls (and its not a datetime/bool/timedelta) then its casted).

This is user initiaded operation (most of the time) because its not exactly cheap to do this

@jreback Because this is an expensive operation, does it make sense to add a flag in the parameters to try convert_objects?

I don't think this should automatically convert; rather you can check if their are non-valid dtypes

right now I think no non-floats are allowed (though in theory it should work).

closing

ithis should raise/warn on object dtype not auto-convert
VIS: let scatter plots obey mpl color scheme (#3338)
State: closed

Closes #3338.

Fixes the remaining plot functions that don't follow the matplotlib style (functions based on scatter plots).

Rationale:
- set the color keyword `c` to default one defined in matplotlibs rcParams if `c` was not specified by the user (because `c='b'` is harcoded in matplotlib `scatter` method).
- the `color` keyword is not checked because, if specified by the user, it already is given preference by matplotlib over `c` if both are given.

There is a new PR on scatter based plotting methods (#3473), but I suppose that it can be handled in that PR to ensure this new plotting method also follows this approach?


Labels:

Comments:
@cpcloud ?

pulling it down now

Still get obnoxious blue:

![funky-colors](https://f.cloud.github.com/assets/417981/1258494/61e199da-2bbf-11e3-81af-db6070f50997.png)

@cpcloud How did you obtain this plot?

If I do (example from the docs):

``` python
pd.options.display.mpl_style = 'default'
df = pd.DataFrame(np.random.randn(1000, 4), index=pd.date_range('1/1/2000', periods=1000), columns=list('ABCD'))
df = df.cumsum()
df.plot()
```

I get the new color scheme, as you also see in the docs: http://pandas.pydata.org/pandas-docs/dev/visualization.html#basic-plotting-plot

@cpcloud Ah, it seems like you posted the plot in this part of the docs: http://pandas.pydata.org/pandas-docs/dev/visualization.html#suppressing-tick-resolution-adjustment

Well, this one is generated by explicitly stating the colors you want (`df.A.plot(color='r')`), so it logically follows this and not the default color scheme.

Off course, we could remove this `color=..` like this:

```
with pd.plot_params.use('x_compat', True):
    df.A.plot()
    df.B.plot()
    df.C.plot()
```

to get rid of these colors in the docs, but then the purpose of the example is a little bit lost, because you could do the same with just `df[['A', 'B', 'C']].plot(x_compat=True)`

@jorisvandenbossche ready to go?

@cpcloud this ok?

@jreback for me this is ready

Yep good 2 go
TST: allow to check for specific xlrd version and skip reader test if xlrd < 0.9
State: closed

See http://nipy.bic.berkeley.edu/builders/pandas-py2.x-wheezy-sparc/builds/148/steps/shell_4/logs/stdio for failure when outdated xlrd found (e.g. on debian stable wheezy)


Labels:

Comments:
@cpcloud @jtratner seems reasaonable....

fine with me. At some point probably better to use LooseVersion, but seems like a good fix.

minimum version I can get to pass the test suite is 0.8.
BUG: fix Index's __iadd__ methods
State: closed

closes #4996

Aliases **iadd** = **add**.

Not doing anything else for now until time to consider.


Labels:

Comments:
@jreback are you sure this doesn't break back-compat? Seems like it would.

@jtratner I think its ok; the problem is iadd was using the numpy iadd which effectively does the same thing, but doesn't do the asignment correctly (because its a numpy method and doesn't go thru the python assignment machinery)

I just don't think you should block the non inplace methods without
deprecating them first. It's not unreasonable to know that Index is a thin
wrapper around ndarray, etc.

they don't work properly is the issue as they update the Index inplace which is semantically wrong (because its not calling the right method).

I think it's fine to block inplace, but I thought you were blocking things
like **mul**?

@d10genes ok...this got too complicated! sorry.....let's just do the iadd; rest can do in a separate PR

iadd...and isub? or just iadd

Just `__iadd__`. Leaving implementing any _new_ features for later.

I went ahead and updated the title of this PR to make it clearer what's going on. We definitely appreciate you being game on this - please just remove the `__isub__` and `__sub__` and then squash your commits. Should be ready to merge at that point.

If you want, you could open a new pull request with your proposed `__sub__` implementation.

thanks you!
ENH/DOC: Cleanup docstrings on NDFrame
State: closed

Resolves much of #4717. Basically cleans up docstrings in a few places
plus, for the few functions that need them, uses a shared dict of
function definitions to share them. This has the side-benefit of letting
the function docstrings reside right next to the functions themselves,
maintaining readability.


Labels:

Comments:
@jreback what do you think? I think this is nicely succinct and easy to follow.

find by me (just those clipboard comments) above

so it's just pyperclip for OSX/Windows?

pyperclip IIRC tries various methods eg xclip,xsel, qt on Linux; native access in windows and says it works on Mac which I think was verified 

it is included in pandas/utils IIRC
CLN: Fixups to exceptions in generic
State: closed

Improved message and better exception types.


Labels:

Comments:
@jtratner this still relevant? close?

I'll either close or fix this tonight.
On Oct 8, 2013 7:47 AM, "jreback" notifications@github.com wrote:

> @jtratner https://github.com/jtratner this still relevant? close?
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/5051#issuecomment-25883023
> .

this looks fine....merge at your leisure

@jtratner closing this?

I'll push it to 0.14 - I think there's one item from this I want to include.

np.
CLN/BUG/TST: Fix and test multiple places using undefined names.
State: closed

Pyflakes revealed that there are a number of places using undefined
names. I've tried to fix them where I can; however, those parts of the
code are also clearly untested (or tested with
`assertRaises(Exception,...)`) so they need some love.

The PyTables code had a bunch of places with undefined `TermValue`, as
did stats/misc.

Finally, I've also noted (or removed) various places where variables
were assigned but then not used.

cc @jreback


Labels:

Comments:
@jtratner when you have a chance rebase this....and can put it in

Well, the main goal here was to point out where we don't have test coverage
(and could even be unreachable code, depending on how long the code has
been around). Going to see which areas are fixed after rebasing and which
still need to be fixed.

@jtratner looks ok to put in...?

I don't expect this PR to be merged, goal was to find places where there's untested code (so eventually everything here should be addressed separately). I recall waiting to hear back on the date start/stop stuff, I'll take another look and see. You covered the pytables elements already, which is good.

Actually can you check that all the pytables code referenced with comments, (i.e., the TODOs or not used parts) are either removed or now have tests in master?  Last I checked, the issues with `data` and `self.data` being wrong still weren't fixed. And we really need to add tests that hit those branches (and then fail because of the NameErrors)

@jtratner so....let's just move to 0.14...(or you can create an issue to remind to do this kind of analysis )

If they're legacy code, then I guess we can just fix the NameErrors (didn't
realize that was what was going on) and move on. That said, unless we
introduced those changes since v0.12, no one is using those branches.
On Oct 11, 2013 8:28 AM, "jreback" notifications@github.com wrote:

> @jtratner https://github.com/jtratner so....let's just move to
> 0.14...(or you can create an issue to remind to do this kind of analysis )
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/5045#issuecomment-26133245
> .

yep....it was actually pretty tricky to come up with a case where the original code failed...you actually have to reset the timezone in python to do it. Bottom line is that these guys are using `datetime.date` which is pretty much unsupported, so wasn't a big deal.

Okay, sounds good. A couple more things that need fixing and I'll take a
look and ping the appropriate people :)

Maybe leave a note on the tests that they're testing deprecated  behavior
and can be removed after some version you specify?

well..its not really deprecated per se, just generated differently in current version that prior version (in theory I should actually bump the version number of the generated HDFStore files, but since it can be handled both ways its not necessary)

@jtratner anything left on this?

@jtratner ?

@jtratner ?

at this point, I'm assuming this can be closed.

ok...