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

Wip update spire #833

Merged
merged 3 commits into from
Jan 27, 2022
Merged

Wip update spire #833

merged 3 commits into from
Jan 27, 2022

Conversation

Quafadas
Copy link
Contributor

This aims to remove the 2_13 dependancy on spire. Spire itself was compiled with 3.1.0 as far as I could tell, so I also bumped breezes scala version.

That generated a bunch of problems. Upgrading the collection-compact modules, apparently fixed these problems.

With these changes, the tests in the "math" sub project passed (for me).

Health Warnings:
Spire release is "M2". It may be preferable to wait for a 0.18.0
As I don't have a detailed understanding of Breeze, these changes may not be desirable in some larger context I'm not aware of.

@dlwh
Copy link
Member

dlwh commented Dec 31, 2021

Thanks! I have a slight preference for supporting 3.0.x, but if that's what Spire can support, than probably best to bump. I agree it's best to wait for a full release or at least RC before we cut a release with it.

@armanbilge
Copy link

Just had a spire user run into the dreaded Conflicting cross-version suffixes trying to use spire and breeze together on Scala 3 and found my way here. Thanks for working on this!

I have a slight preference for supporting 3.0.x

Thanks, this is helpful feedback. AFAIK nothing in spire depends on Scala 3.1.x (but can't be sure), so we can try rolling back if it's important to you. This issue has been controversial :) We could also wait and see about scala/scala3#14156, I think they originally promised it for 3.1.1 but I haven't followed closely.

I agree it's best to wait for a full release or at least RC before we cut a release with it.

We released an M3 last week. I don't anticipate any more breaking changes going forward, we can probably RC or go straight to final.

@romanowski
Copy link

romanowski commented Jan 13, 2022

scala/scala3#14156 will be released in 3.1.2. RC1 should be released shortly and a stable version 6 weeks later (unless there are no critical bugs).

@Swoorup
Copy link

Swoorup commented Jan 14, 2022

@dlwh
Copy link
Member

dlwh commented Jan 27, 2022

sorry for being slow. Yeah my preference for 3.0.x was driven by the lack of forwards compatibility, but if they're going to fix that, then that's great.

@dlwh dlwh merged commit ee30ba4 into scalanlp:master Jan 27, 2022
dlwh added a commit that referenced this pull request Jan 27, 2022
This reverts commit ee30ba4, reversing
changes made to 0f3d9e8.
@dlwh
Copy link
Member

dlwh commented Jan 27, 2022

oops. shouldn't merge things when i have a migraine

@dlwh
Copy link
Member

dlwh commented Jan 27, 2022

(reverted the merge in master. don't know how to reopen a PR)

@armanbilge
Copy link

sorry for being slow. Yeah my preference for 3.0.x was driven by the lack of forwards compatibility, but if they're going to fix that, then that's great.

@dlwh do you still feel the same way about sticking to Scala 3.0.x? I think Spire will go straight to Scala 3.1. I experimented with the new -scala-output-version flag but it seems we've already used some Scala 3.1 APIs in macros so it doesn't work. I'd rather not backport these.

[error] -- Error: /workspace/spire/core/src/main/scala-3/spire/syntax/macros/literalMacros.scala:35:14 
[error] 35 |  parseNumber(digits.valueOrAbort.parts, BigInt(-128), BigInt(255)) match
[error]    |              ^
[error]    |method valueOrAbort was added in Scala release 3.1, therefore it cannot be used in the code targeting Scala 3.0

See some discussion on the topic in:

@dlwh
Copy link
Member

dlwh commented May 26, 2022 via email

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.

5 participants