Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Background Map for Plotting #170
base: master
Are you sure you want to change the base?
Background Map for Plotting #170
Changes from all commits
702b263
0cd6f7e
4b5442a
de0d96a
f757d42
852c0d0
d1350b2
b165624
29d8d46
3bc629c
a906db6
03ec548
fb9972c
78a5ff7
294b12c
95f4061
aba9ad3
6120562
d08d9f6
2ac8cb6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 13 in ext/QuantumSavoryGeoMakie/QuantumSavoryGeoMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryGeoMakie/QuantumSavoryGeoMakie.jl#L11-L13
Check warning on line 22 in ext/QuantumSavoryGeoMakie/QuantumSavoryGeoMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryGeoMakie/QuantumSavoryGeoMakie.jl#L15-L22
Check warning on line 27 in ext/QuantumSavoryGeoMakie/QuantumSavoryGeoMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryGeoMakie/QuantumSavoryGeoMakie.jl#L25-L27
Check warning on line 290 in ext/QuantumSavoryMakie/QuantumSavoryMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryMakie/QuantumSavoryMakie.jl#L290
Check warning on line 294 in ext/QuantumSavoryMakie/QuantumSavoryMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryMakie/QuantumSavoryMakie.jl#L293-L294
Check warning on line 297 in ext/QuantumSavoryMakie/QuantumSavoryMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryMakie/QuantumSavoryMakie.jl#L296-L297
Check warning on line 305 in ext/QuantumSavoryMakie/QuantumSavoryMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryMakie/QuantumSavoryMakie.jl#L305
Check warning on line 308 in ext/QuantumSavoryMakie/QuantumSavoryMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryMakie/QuantumSavoryMakie.jl#L307-L308
Check warning on line 310 in ext/QuantumSavoryMakie/QuantumSavoryMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryMakie/QuantumSavoryMakie.jl#L310
Check warning on line 314 in ext/QuantumSavoryMakie/QuantumSavoryMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryMakie/QuantumSavoryMakie.jl#L313-L314
Check warning on line 320 in ext/QuantumSavoryMakie/QuantumSavoryMakie.jl
Codecov / codecov/patch
ext/QuantumSavoryMakie/QuantumSavoryMakie.jl#L317-L320
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workaround of using
get_extension
is something I set up for use with types and structs, but it is not a very good way to deal with functions, given the multimethod nature of julia.Methods of functions can be added in extensions -- that would be much more natural than what we are doing here, of using a new function in a new namespace.
For now we can probably keep this implementation, but it would be important to create an issue to track fixing this hack.
Potentially a better way to do this would be to have a hint attached to method errors. Something like what is done in this PR https://github.com/QuantumSavory/QuantumClifford.jl/pull/416/files# but just warning that the method error happened because the necessary package was not imported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, it seems it is worse than that. Currently the way you have namespaced things it looks like a method is being overridden instead of a new function being made. That can be easily fixed by just restricting what is being imported in the namespace and keeping your current implementation.
But given that things need to be fixed anyway, I suggest doing the proper fix with method error hints that I described above.
Here is the errors I am currently getting
Check warning on line 7 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L4-L7
Check warning on line 9 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L9
Check warning on line 18 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L15-L18
Check warning on line 20 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L20
Check warning on line 29 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L26-L29
Check warning on line 31 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L31
Check warning on line 40 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L37-L40
Check warning on line 42 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L42
Check warning on line 56 in src/plots.jl
Codecov / codecov/patch
src/plots.jl#L56