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

various package status improvement #8

Merged
merged 3 commits into from
Nov 5, 2021

Conversation

johnnychen94
Copy link
Collaborator

Instead of doing JuliaImages/ImageFiltering.jl#235 I think it might be worth improving this package first 😆

This PR doesn't touch the core functionality with one exception rayleighmode reworked using ImageContrastAdjustment.

Depending on how you want this package to work closely with the JuliaImages ecosystem, I can contribute more to e.g., improve test codes, docstrings, and potentially improve the core implementation when I'm confident.

@peterkovesi
Copy link
Owner

Thank you for all the work you have done. Getting the dependencies down is a good thing. However, it looks like AbstractFFTs needs to be reinstated (or it is being unnecessarily used somewhere)
I was not familiar with your DemoCards package, looks neat.

You are most welcome to make improvements to this package as you wish. I am delighted that you are interested in the package. On my own all I would be doing is trying to keep compat entries maintained and otherwise doing the bare minimum to keep the package useable.

Images is a big dependency that most package developers should not
use directly. Instead they're encouraged to use exactly the ones
they need.

There are some test and documentation related dependencies. This
commit removes them.
@johnnychen94
Copy link
Collaborator Author

However, it looks like AbstractFFTs needs to be reinstated (or it is being unnecessarily used somewhere)

Right, I missed committing one change. It seems that this repo has messed up with whitespace issues. VSCode makes a bunch of auto-fixes and it makes tracking changes quite hard 😂

screenshot

image

Can you run some whitespace fixes on the git history? For instance, the ws-fix.sh in https://stackoverflow.com/questions/18076263/remove-historical-trailing-white-spaces-in-git works quite well. I also made a cleaned version in https://github.com/johnnychen94/ImagePhaseCongruency.jl/tree/master-ws-fix


DemoCards is used to support all these kinds of examples and tutorials. It follows a mixed syntax of Documenter and Literate.jl. Currently, we need to wait for Images v0.25 out first. But if you're interested to check it locally, you can try to add JuliaImages/Images.jl#971 branch.


I'm definitely interested in improving the status of this package; you're really an expert on this and I think people should be grateful to use them. I'm also considering making this package an external extension to ImageFiltering, or forking the functionality into ImageFiltering. I'm not sure yet which is better, but I do want to advertise your packages in JuliaImages. Or if you want to make them exposed more, you're welcome to transfer your packages to JuliaImages.

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@9cc9dbd). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master       #8   +/-   ##
=========================================
  Coverage          ?   87.45%           
=========================================
  Files             ?        5           
  Lines             ?      765           
  Branches          ?        0           
=========================================
  Hits              ?      669           
  Misses            ?       96           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9cc9dbd...ecafeaf. Read the comment docs.

@peterkovesi peterkovesi merged commit 86f3587 into peterkovesi:master Nov 5, 2021
@johnnychen94 johnnychen94 deleted the jc/slim branch November 5, 2021 14:17
@johnnychen94
Copy link
Collaborator Author

Thanks for the invitation!

@peterkovesi
Copy link
Owner

I am not sure about the whitespace fixing. All I know is the basics of git - just enough to get me into trouble!
I have invited you to be a collaborator. If it is OK with you you can perform the whitespace fixing.
Sorry not to be participating much at the moment I am busy with other things - I am about to head off on a long sailing trip!

@johnnychen94
Copy link
Collaborator Author

Enjoy the trip and your retired life 😆

johnnychen94 pushed a commit that referenced this pull request Nov 5, 2021
various package status improvement
@johnnychen94
Copy link
Collaborator Author

johnnychen94 commented Nov 5, 2021

If it is OK with you you can perform the whitespace fixing.

Done. You'll have to do a force-reset on your local repo to get the updated history:

git fetch --all
git checkout master
git reset --hard origin/master

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