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

Packaging dependency requirement #2216

Closed
greglucas opened this issue Jul 17, 2023 · 4 comments · Fixed by #2197
Closed

Packaging dependency requirement #2216

greglucas opened this issue Jul 17, 2023 · 4 comments · Fixed by #2197

Comments

@greglucas
Copy link
Contributor

Description

packaging is only listed as a test dependency, but we are now version gating in the main importable code section. We should either figure out a different way to do the version checks within the code or add it as a primary dependency.

ping @rcomer it looks like this came in these two PRs: #2213, #2215

@greglucas greglucas added this to the 0.22 milestone Jul 17, 2023
@rcomer
Copy link
Member

rcomer commented Jul 17, 2023

I think I started it with #2169, with more in #2166 and I was copying how it was done in the tests. Before that the version checks in the main code were just comparing strings, but I think we need something more sophisticated than that once mpl 3.10 comes out

In [1]: "3.10" > "3.4"
Out[1]: False

@rcomer
Copy link
Member

rcomer commented Jul 18, 2023

I have to admit it did not cross my mind to think about what type of dependency this is. Sorry about that.

@dopplershift
Copy link
Contributor

I don't think it's a particularly burdensome dependency, but it is definitely should be added to the install list.

@greglucas
Copy link
Contributor Author

greglucas commented Jul 19, 2023

I have to admit it did not cross my mind to think about what type of dependency this is. Sorry about that.

No problem at all @rcomer! I just wanted to make sure we had a plan for addressing this. I'll add this as a dependency in my release PR so we don't have to deal with rebase conflicts when adding it to current main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants