Skip to content

Refactor inverse_eltype calculations to use types. #137

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

Merged
merged 9 commits into from
May 2, 2025

Conversation

tpapp
Copy link
Owner

@tpapp tpapp commented Apr 28, 2025

This is a continuation of #133 in a sense, using the approach to fix some other issues.

Incidental changes:

tpapp added 6 commits April 28, 2025 15:27
Incidental changes:

- whitespace and formatting
- clarify that inverse_eltype does (may) not do input checks
- fixed #73, the corner case of the empty vector; tests added
- rename utility function introduced in #133 and place in utilities.jl
@tpapp tpapp requested a review from devmotion April 28, 2025 15:19
Copy link
Collaborator

@devmotion devmotion left a comment

Choose a reason for hiding this comment

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

I'll review in more detail later today or tomorrow. Generally, it looks fine but this is a breaking change (inverse_eltype is part of the documented interface and with this PR inverse_eltype of TransformTuple etc call the definition with types which mostly likely downstream packages do not define currently and hence will throw an error).

@tpapp
Copy link
Owner Author

tpapp commented Apr 29, 2025

Yes, of course it is a breaking change. That's why I released 1.0.0 yesterday, instead of relying on "you can break everything in 0.x".

Among registered packages, the following are affected because they define a method (based on dependents list and code search):

If we release 2.0.0, we might as well fix #66 since that is breaking too.

@tpapp
Copy link
Owner Author

tpapp commented May 2, 2025

I decided pull back the 1.0.0 release, release this as 0.9.0, and fix other breaking changes before 1.0.0.

@stjordanis, I am pinging you here since the above Pumas.jl repo does not accept issues.

@devmotion
Copy link
Collaborator

You don't have to worry about that repo, it's a completely outdated version of Pumas that was copied by the Github user when its source code was publicly available.

@tpapp tpapp merged commit d8f4a9e into master May 2, 2025
4 of 6 checks passed
@tpapp tpapp deleted the tp/inverse_eltype_cleanup branch May 2, 2025 09:21
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.

Inverse transformation for empty vectors fails
2 participants