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

Python version support #40

Merged
merged 10 commits into from
Aug 22, 2022
Merged

Python version support #40

merged 10 commits into from
Aug 22, 2022

Conversation

itepifanio
Copy link
Collaborator

@itepifanio itepifanio commented Aug 15, 2022

Fix #36 and #37

Improve Ipyannotator python support and fix setup.py requirements.

@ibayer
Copy link
Member

ibayer commented Aug 16, 2022

tests for py 3.8-3.10 are run in ci and passing
https://github.com/itepifanio/ipyannotator/actions/runs/2863251240

@ibayer
Copy link
Member

ibayer commented Aug 16, 2022

closes #37

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated
Comment on lines 12 to 21
pandas = "^1.1.2"
traitlets = "^5.1.1"
ipycanvas = "^0.10.2"
ipyevents = "^0.8.0"
ipykernel = "^5.3.4"
ipywidgets = "^7.5.1"
scikit-image = "^0.18.3"
scikit-image = "^0.19.3"
matplotlib = "^3.4.3"
pydantic = "^1.8.2"
PyPubSub = "^4.0.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these requirements are over restrictive and contribute to the problems I mentioned in Issue #36. Unless you have specific reasons why you need to be restricting these libraries down to the patch level you are making it so that it is very difficult to use your software as anything but an application, and the documentation makes it seem like you want people to be using it along with their other code so that it can be useful in a workflow.

Copy link
Member

Choose a reason for hiding this comment

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

@matthewfeickert

Thanks for this great comment. I believe that poetry add caps requirements at patch level by default.
In any case, we don't cap our requirements for libraries by default when we set them manually. In the future we'll make sure to double check versioning when using tooling that creates/manages requirements.

@AlexJoz fyi

@itepifanio I agree with @matthewfeickert suggestion to switch from version cap to setting minimum version by default (also true for dev req.). I don't think we need to cap any of the ipyannotator requirements, but I'm not 100% sure.

Copy link
Member

@ibayer ibayer left a comment

Choose a reason for hiding this comment

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

@itepifanio
Thanks for sorting out the versions! We can merge if you fix my two minor requests.

.github/workflows/release.yaml Outdated Show resolved Hide resolved
.github/workflows/release.yaml Outdated Show resolved Hide resolved
@itepifanio
Copy link
Collaborator Author

@itepifanio Thanks for sorting out the versions! We can merge if you fix my two minor requests.

Done @ibayer

@ibayer ibayer merged commit 6e01e8c into palaimon:main Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[JOSS Review] Python version support and clarification of framework vs. application
3 participants