deleted unused param "session" from process_split

[smellslikekeenspirit]

References to other Issues or PRs

Brief description of what is fixed or changed

Other comments

Release Notes

• core
  • removed unused parameter from process_split function

[sympy-bot]

✅

Hi, I am the SymPy bot (v145). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

• core
  • removed unused parameter from process_split function (#16594 by @smellslikekeenspirit and @smichr)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.5.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed


#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* core
    * removed unused parameter from process_split function

<!-- END RELEASE NOTES -->

[codecov]

Codecov Report

Merging #16594 into master will increase coverage by 0.047%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##            master    #16594       +/-   ##
=============================================
+ Coverage   73.719%   73.766%   +0.047%     
=============================================
  Files          619       619               
  Lines       158731    158731               
  Branches     37207     37207               
=============================================
+ Hits        117015    117091       +76     
+ Misses       36301     36234       -67     
+ Partials      5415      5406        -9

[smichr]

I'm not sure that this change should be made since line 62 calls this using session. On the other hand, both functions were added in 2673cc4 so maybe it could be deleted from both places. I don't have a lot of experience with the testing files, so I don't have a good sense for how this is used and whether some other testing platform will call these functions. Perhaps @asmeurer , @oscarbenjamin or @oscargus would have some ideas on this.

There will be no controversy for something like #16598, however.

[oscargus]

No idea, but I share your analysis regarding line 62. One should probably check if pytest_collection_modifyitems is called (the comment, """ pytest hook. """ seems to indicate that, if nothing else, session should be kept in that method, but not passed on). However, since the tests pass as is, with what seems like valid splits, it seem like maybe pytest_collection_modifyitems is not needed at all (and therefore, maybe, just maybe, not process_split either).

[oscarbenjamin]

git grep reveals that these are the only two places the function is referred to:

$ git grep process_split
sympy/conftest.py:def process_split(session, config, items):
sympy/conftest.py:    process_split(session, config, items)

So I think it's fine to change it provided it is changed in the call as well as the signature.

Note that in general some files used by pytest can have functions called pytest_* that are magic functions picked up by pytest. So even if they are never called in the SymPy code that argument could still be needed, I don't think that's the case here though.

[smichr]

So it looks like all that is needed is that the session in line 62 be deleted, too.

[oscarbenjamin]

Your new commits include unrelated files. Can you remove them please?

[smellslikekeenspirit]

Why are the deletions rejected by travis ci?

[smichr]

Why are the deletions rejected by travis ci?

I am not sure why this is happening. Something that makes your situation a little more difficult to fix is that you used the master branch to make the changes. The workflow suggestion is that you create a new branch when starting an issue.

Since this PR can only be updated with your master branch, the only way to fix things is to open a new PR. I copied your commits and did so at #16614. That PR will close this when it is committed.