Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Magics.parse_options parse problem #12107

Closed
louismartin opened this issue Jan 31, 2020 · 3 comments
Closed

Magics.parse_options parse problem #12107

louismartin opened this issue Jan 31, 2020 · 3 comments

Comments

@louismartin
Copy link

Hi,
I think there is a bug with the parse_options method in the sense that it modifies the input statement which causes a bug in my application.

Repro

In jupyter notebook, when iterating over my dataframe, I tried to time the operation with the timeit magic:

> df = pd.DataFrame({'document': ['First doc.', 'Second doc.']})
> %timeit _ = [s.count('. ') for s in df['document'].tolist()]
KeyError: ' document'

Note the key that was transformed from 'document' in my code to ' document' at execution time.

This happens in the parse_options method

opts, stmt = self.parse_options(line,'n:r:tcp:qo',

        opts, stmt = self.parse_options(line,'n:r:tcp:qo',
                                        posix=False, strict=False)

When dropping into pdb, I get the following:

> print(line)
_ = [s.count('. ') for s in df['document'].tolist()]
> print(stmt)
_ = [s.count('. ') for s in df[' document'].tolist()]

and within parse_options(), I guess it happens when splitting the line with arg_split and then joining back with spaces

> argv = arg_split(arg_str, posix, strict)
> print(arg_str)
_ = [s.count('. ') for s in df['document'].tolist()]
> print(argv)
['_', '=', "[s.count('.", "') for s in df['", "document'].tolist()]"]

And thenargs = ' '.join(args) will join with an extra space.

Also changing my command from _ = [s.count('. ') for s in df['document'].tolist()] to _ = [s for s in df['document'].tolist()] removes the problem so there might be a problem with the .count('. ').

@Carreau
Copy link
Member

Carreau commented Jan 31, 2020

Hum, thanks, that seem legit. This code is really nasty; and it might take some time to fix.
Pull requests appreciated.

@adityausathe
Copy link
Contributor

#12708
I have raised a PR to address this issue. @Carreau please review.

@Carreau Carreau added this to the 8.0 milestone Apr 17, 2021
Carreau added a commit that referenced this issue Apr 17, 2021
Issue #12107 fix: additional spaces while parsing timeit-magic options
@ivanov
Copy link
Member

ivanov commented Jun 24, 2021

Hey there, I'm going through old issues and it seems to me that it makes sense to close this one.

The fix for this was merged in #12708 and will make it into IPython 8.0 🎉

Thanks everyone and happy hacking! :bowtie:

@ivanov ivanov closed this as completed Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants