Skip to content

Change clenshaw!/horner! to mutate first argument #255

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
Jan 9, 2025

Conversation

dlfivefifty
Copy link
Member

To make the interface for clenshaw! more consistent with sum!, cumsum!, diff! the vector being mutated should be first.

This goes with JuliaApproximation/RecurrenceRelationships.jl#13 which also supports the dims keyword

Copy link

codecov bot commented Jan 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.12%. Comparing base (5260921) to head (0af65e9).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #255   +/-   ##
=======================================
  Coverage   77.12%   77.12%           
=======================================
  Files          17       17           
  Lines        3156     3156           
=======================================
  Hits         2434     2434           
  Misses        722      722           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dlfivefifty
Copy link
Member Author

I disabled examples/semiclassical.jl testing as it depends on ApproFun.jl which makes a circular problem for testing

@MikaelSlevinsky
Copy link
Member

I disabled examples/semiclassical.jl testing as it depends on ApproFun.jl which makes a circular problem for testing

Why is disabling this necessary? ApproxFun is only a requirement in the docs/Project.toml, so not for loading the package.

@dlfivefifty
Copy link
Member Author

The documentation tests break since it tries all examples and there’s a circular dependency

@MikaelSlevinsky
Copy link
Member

I didn't realize there were documentation tests... Where are they? The latest commit had a checkmark (5260921), so nothing that's being tested or built failed.

@MikaelSlevinsky
Copy link
Member

MikaelSlevinsky commented Jan 7, 2025

Oh I see that when you bumped the version to 0.17 (denoting breaking change) that it was unable to load ApproxFun due to its dependency restriction. (https://github.com/JuliaApproximation/FastTransforms.jl/actions/runs/12596799219/job/35108595588#step:4:54). There must be a way around this... because it basically works for patch releases.

@MikaelSlevinsky
Copy link
Member

Maybe the example should live in https://github.com/JuliaApproximation/ApproxFunExamples

@MikaelSlevinsky
Copy link
Member

If the docs fail to build, the PR can still be merged. A new minor release will prompt ApproxFun to bump its patch release, which can then allow the docs to be rebuilt here... The only thing would be merging with a x instead of a ✓.

@dlfivefifty
Copy link
Member Author

Then it wouldn’t have caught the actual bugs in the documentation.

moving it to ApproxFunExamples makes more sense

@dlfivefifty
Copy link
Member Author

I also removed Reexport since it's bad practice (technically makes any breaking change in other packages a breaking change here)

@jishnub
Copy link
Member

jishnub commented Jan 9, 2025

Probably this fixes #213?

@dlfivefifty dlfivefifty merged commit a40b3dc into master Jan 9, 2025
20 checks passed
@dlfivefifty dlfivefifty deleted the dl/clenshawargorder branch January 9, 2025 11:55
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.

3 participants