Skip to content

added sys import - #7911

Merged
sklam merged 3 commits into
numba:mainfrom
Nightfurex:main
Mar 16, 2022
Merged

sklam merged 3 commits into
numba:mainfrom
Nightfurex:main

Conversation

@Nightfurex

Copy link
Copy Markdown
Contributor

No description provided.

@Nightfurex
Nightfurex requested a review from DrTodd13 as a code owner March 15, 2022 15:30
@DrTodd13

Copy link
Copy Markdown
Contributor

In the original file on lines 325 and 326 is the only place where "sys" is used. Those are really debugging code that should be removed so my preferred solution is to remove those rather than adding a sys import. Can you make that change please?

@DrTodd13 DrTodd13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment about removing the line that uses sys.

@Nightfurex

Copy link
Copy Markdown
Contributor Author

Ooh sorry my bad i misunderstood .

exp_name_to_tuple_var)
if config.DEBUG_ARRAY_OPT:
sys.stdout.flush()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should only be one blank line here.

@DrTodd13

Copy link
Copy Markdown
Contributor

Thanks for making this change. Just that one additional picky thing about the extra line and I think we're done.

@DrTodd13 DrTodd13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@stuartarchibald

Copy link
Copy Markdown
Contributor

Thanks for the patch @Nightfurex, thanks for reviewing @DrTodd13.

@stuartarchibald stuartarchibald added the 5 - Ready to merge Review and testing done, is ready to merge label Mar 15, 2022
@stuartarchibald

Copy link
Copy Markdown
Contributor

Note: I've manually tested against the reproducer in #7910.

@stuartarchibald stuartarchibald added this to the Numba 0.56 RC milestone Mar 15, 2022
@stuartarchibald stuartarchibald added the Effort - short Short size effort needed label Mar 15, 2022
@stuartarchibald stuartarchibald linked an issue Mar 15, 2022 that may be closed by this pull request
@sklam
sklam merged commit a435578 into numba:main Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Review and testing done, is ready to merge Effort - short Short size effort needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing sys import in parfor_lowering

4 participants