Skip to content

MAINT: lib: Some code clean up in loadtxt - #16633

Merged
mattip merged 4 commits into
numpy:masterfrom
WarrenWeckesser:maint-loadtxt
Jun 18, 2020
Merged

MAINT: lib: Some code clean up in loadtxt#16633
mattip merged 4 commits into
numpy:masterfrom
WarrenWeckesser:maint-loadtxt

Conversation

@WarrenWeckesser

@WarrenWeckesser WarrenWeckesser commented Jun 18, 2020

Copy link
Copy Markdown
Member

This is a maintenance-only PR, in preparation for some bug fixes and enhancements.

@WarrenWeckesser
MAINT: lib: Move some nested function definitions in loadtxt.
be11d11 
This change moves the nested function definitions in loadtxt
to the top of the function body.
@WarrenWeckesser
MAINT: lib: In loadtxt, validate ndmin argument earlier.
2d31d06 
Validation of `ndmin` is moved to the beginning of the function,
so we don't read the entire file only to raise an exception at
the end because of a bad argument.
@WarrenWeckesser
MAINT: lib: In loadtxt, move some code out of a try/finally block.
fa1a3ec 
In loadtxt, there is a try/finally block that ensures that the file
is closed if it was opened in the function.  Some code that did not
need to be in that block was moved up, outside the try/finally block.
@WarrenWeckesser
MAINT: lib: A few PEP-8 fixes.
431f0ef
@WarrenWeckesser WarrenWeckesser mentioned this pull request Jun 18, 2020
Closed
@WarrenWeckesser

WarrenWeckesser commented Jun 18, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

I was tempted to move the nested functions flatten_dtype_internal and pack_items out of the loadtxt definition, and make them private functions at the top level of the module. Having them nested in loadtxt is unnecessary. They are standalone functions, with no need to be exposed to the scope of loadtxt. If anyone agrees such a change is worthwhile, I'll add it to this PR.

eric-wieser
eric-wieser approved these changes Jun 18, 2020
View reviewed changes

@eric-wieser eric-wieser left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks for splitting into separate commits too!

@WarrenWeckesser

WarrenWeckesser commented Jun 18, 2020

Copy link
Copy Markdown
Member Author

The one failure on TravisCI is the s390x platform, where the log has
just one line:

An error occurred while generating the build script.

@mattip
mattip merged commit b3f7132 into numpy:master Jun 18, 2020
@mattip

mattip commented Jun 18, 2020

Copy link
Copy Markdown
Member

Thanks @WarrenWeckesser

@WarrenWeckesser
WarrenWeckesser deleted the maint-loadtxt branch June 18, 2020 14:16
@rossbar rossbar mentioned this pull request Jun 29, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eric-wieser eric-wieser eric-wieser approved these changes

Assignees

No one assigned

Labels

03 - Maintenance component: numpy.lib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@WarrenWeckesser @mattip @eric-wieser