CI: Check for inconsistent pandas namespace usage #37188
Conversation
dsaxton
added
CI
There was a problem hiding this comment.
yeah we had an issue where we talked about this a while back . There was an analysis and IIRC we were basically split between these usages. The discussion basically said that an individual file should be consistent (either pd.* or just Series/DataFrame), but we wouldn't do the effort to clean everything up.
@jorisvandenbossche was for not clean
me and @jbrockmendel though we should just standarize on Series/DataFrame
IIRC.
I am still in favor of using Series/DataFrame everywhere in tests (and not pd.*)
|
I agree with @jreback: small preference towards not |
|
yeah it think this is ok to add. will have to figure out the best way to expand this coverage. maybe a file for includes with the test files to check? (or a config like file). |
The diffs will be very large but it may be easier just to fix this globally for the different classes. I did this for Series just now (can revert if the PR is too large) and could do for DataFrame, etc. if worthwhile. |
oh i think this is fine (yeah do DataFrame separately); only issue is have to make sure all passing (in case we are missing imports). ci is a bit cranky now. |
|
and actually merge master once again (as just merged big PR). ping when you are ready. |
Looks to be mostly green (only annoying thing that can happen are false positives due to Series(...) being used in comments but not code, so I changed those as well) |
|
thanks @dsaxton |
* CI: Check for inconsistent pandas namespace usage * Make a function * Remove * Try making it fail * Add message * Fix * Make pass * Try something * Remove sed * Edit * Switch file * Revert * Global Series fix * More * Remove
* CI: Check for inconsistent pandas namespace usage * Make a function * Remove * Try making it fail * Add message * Fix * Make pass * Try something * Remove sed * Edit * Switch file * Revert * Global Series fix * More * Remove
Adding a CI check that we aren't (for instance) using Series(...) and pd.Series(...) in the same file.
This is kept intentionally small in scope (checking only DataFrame and Series for one file name right now) since this is very common in the code base and I'm not sure if this is something we'd actually want to enforce.