-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add Aqua tests and resolve method ambiguities #119
Conversation
This commit significantly narrows the types available for the first argument in `convert_arguments(::T1, ::Table)`. This removes a method ambiguity I ran into when testing Shapefile.jl on my machine.
CI failures are unrelated and fixed in #118. |
Can we lock this in with Aqua.il tests? |
I guess? Not sure how to test only one function though. |
I mean test the whole thing! Maybe there are no other ambiguities and its fine? Otherwise sure we can merge it. But you can ignore some functions or modules in Aqua ambiguity testing if you need to. |
There was only one ambiguity from Shapefile.jl proper which is pretty nice :D Thanks for the tip, I implemented Aqua but ignored all the deps compat stuff for now. Tests should be green! |
scatter(::Table)
convert arguments.
Ah these tests are failing here too, I thought it was just an issue on my laptop. Not sure what to do about these, they don't seem catalyzed by any change here. https://github.com/JuliaGeo/Shapefile.jl/actions/runs/10312403712/job/28547583460?pr=119#step:5:831 |
This is mainly so that the overview is better looking.
Yes good to see. Weird that test has broken, I wonder what changed. |
This commit significantly narrows the types available for the first argument in
convert_arguments(::T1, ::Table)
. This removes a method ambiguity I ran into when testing Shapefile.jl on my machine.