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

Which versions of Python should work for p4p? #84

Closed
coretl opened this issue May 23, 2022 · 9 comments
Closed

Which versions of Python should work for p4p? #84

coretl opened this issue May 23, 2022 · 9 comments
Assignees
Labels

Comments

@coretl
Copy link
Collaborator

coretl commented May 23, 2022

p4p has just started giving me test failures on 3.6:

  _______________________________ test_asyncio_ioc _______________________________
  Traceback (most recent call last):
    File "/project/tests/test_asyncio.py", line 49, in test_asyncio_ioc
      assert await ctx.get(pre + ":AI") == 23.45
    File "/tmp/tmp.47L8V0mLtT/venv/lib/python3.6/site-packages/p4p/client/asyncio.py", line 122, in get
      return (await self._get_one(name, request=request))
    File "/tmp/tmp.47L8V0mLtT/venv/lib/python3.6/site-packages/p4p/client/asyncio.py", line 145, in _get_one
      cb = partial(asyncio.get_running_loop().call_soon_threadsafe, cb)
  AttributeError: module 'asyncio' has no attribute 'get_running_loop'

https://github.com/dls-controls/pythonSoftIOC/runs/6551492513?check_suite_focus=true#step:5:601

Docs state 2.7 or 3.4+, setup.py states 2.7+, but get_running_loop() is 3.7+. At DLS we don't use anything less than 3.7 with the epicscorelibs stack, so we would be happy to drop support for 3.6 and down on aioca, pythonSoftIOC, epicscorelibs and p4p. Not sure about other sites.

Alternatively we could revert to asyncio.get_event_loop() or your self.loop that you had before and keep 3.6.

Either way, I think we should change python_requires to be accurate: https://stackoverflow.com/questions/44660448/using-python-requires-to-require-python-2-7-or-3-2

Happy to submit a PR for whatever change you think is best.

@mdavidsaver
Copy link
Member

This is arising from #80 and #82. Which, in my rush to put out a PVXS fix (epics-base/pvxs#27), I admit I forgot about. At minimum a change to compatibility should have triggered 4.1.0 (if not 5.0.0).

As for compatibility notation. This is complicated since there are two different compatibility ranges for the asyncio support, and the rest of P4P. After #82 the asyncio support is 3.7+, while the rest is 2.7, 3.5+ . The only way I could think to represent this in package metadata would be to move the asyncio support to another/new package, which would be a hassle by itself.

Alternatively we could revert to asyncio.get_event_loop() or your self.loop that you had before and keep 3.6.

Happy to submit a PR for whatever change you think is best.

I don't think that I have a preference. If you already have some changes, then by all means please share.

@mdavidsaver
Copy link
Member

I notice that py3.6 has asyncio._get_running_loop(), which does most of the work.

https://github.com/python/cpython/blob/b74b1f36993a4e1700869133f3be13dd60ef4e40/Lib/asyncio/events.py#L638-L646

@coretl
Copy link
Collaborator Author

coretl commented May 23, 2022

How concerned are you with backwards compatibility? Personally I'd be tempted to say we only support 3.7+ for everything, but I don't have any legacy systems on the epicscorelibs stack

@mdavidsaver
Copy link
Member

For me, compatibility with FOSS is a question of effort. When it looks like too much effort, then I'll drop it absent some compelling reason. In this case, after noticing asyncio._get_running_loop() 3.6 support turns out not to be too difficult #86. However, I don't think you should feel compelled to support/test with the full range of versions which I currently support.

@mdavidsaver
Copy link
Member

Also, I'm thinking about creating p4p 4.1.0, and "yanking" 4.0.1 from pypi.org. Thoughts/objections?

@AlexanderWells-diamond

@coretl
Copy link
Collaborator Author

coretl commented May 23, 2022

3.6 support turns out not to be too difficult #86

Looks like a good solution

Also, I'm thinking about creating p4p 4.1.0, and "yanking" 4.0.1 from pypi.org. Thoughts/objections?

Sounds like a good idea to me.

@AlexanderWells-diamond
Copy link
Contributor

I agree with coretl, I'm happy with that solution.

@mdavidsaver
Copy link
Member

Ok, this is done https://pypi.org/project/p4p/#history

@AlexanderWells-diamond
Copy link
Contributor

I can confirm p4p 4.1.0 now passes PythonSoftIOC's Python 3.6 CI:
https://github.com/dls-controls/pythonSoftIOC/actions/runs/2369933404

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

No branches or pull requests

3 participants