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

New package: Kraken v1.2.0 #127181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JuliaRegistrator
Copy link
Contributor

@JuliaRegistrator JuliaRegistrator commented Mar 18, 2025

Copy link
Contributor

github-actions bot commented Mar 18, 2025

Hello, I am an automated registration bot. I help manage the registration process by checking your registration against a set of AutoMerge guidelines. If all these guidelines are met, this pull request will be merged automatically, completing your registration. It is strongly recommended to follow the guidelines, since otherwise the pull request needs to be manually reviewed and merged by a human.

1. New package registration

Please make sure that you have read the package naming guidelines.

2. AutoMerge Guidelines are all met! ✅

Your new package registration met all of the guidelines for auto-merging and is scheduled to be merged when the mandatory waiting period (3 days) has elapsed.

3. To pause or stop registration

If you want to prevent this pull request from being auto-merged, simply leave a comment. If you want to post a comment without blocking auto-merging, you must include the text [noblock] in your comment.

Tip: You can edit blocking comments to add [noblock] in order to unblock auto-merging.

@JuliaRegistrator JuliaRegistrator force-pushed the registrator-kraken-269dd85f-v1.2.0-51e9da3c70 branch from 3a5fd07 to 7d5a68d Compare March 18, 2025 21:56
UUID: 269dd85f-649c-4475-a373-cd87bf6afec3
Repo: https://github.com/vardister/Kraken.jl.git
Tree: 8d15abf20a6cf4084f76e3dfa680095b2035f00b

Registrator tree SHA: 17aec322677d9b81cdd6b9b9236b09a3f1374c6a
@goerz
Copy link
Member

goerz commented Mar 18, 2025

[noblock] How does this compare to https://github.com/org-arl/AcousticsToolbox.jl? @mchitre

@vardister
Copy link

vardister commented Mar 18, 2025

[noblock]

@goerz @mchitre
https://github.com/org-arl/AcousticsToolbox.jl includes support for running a Fortran version of the Kraken program through writing and reading files to and from the program.

Here is a version of Kraken entirely written in Julia, with extra support for calling the Fortran version through ccall.

Some parts of this program are also inspired by https://github.com/org-arl/UnderwaterAcoustics.jl, and credit will be given once I finish writing the README file.

@JuliaTagBot JuliaTagBot added the AutoMerge: last run blocked by comment PR blocked by one or more comments lacking the string [noblock]. label Mar 18, 2025
@mchitre
Copy link

mchitre commented Mar 19, 2025

[noblock]

@vardister Would it be possible to implement the model API in UnderwaterAcoustics.jl in Kraken.jl? That way it can become a model that can easily swapped in by anyone who already uses UnderwaterAcoustics.jl? We have some changes to the API in the upcoming version, so I could work with you on this.

I noticed that you have so and dll files checked into your repo. This may make things tricky to support different architectures and OSs. You may want to consider using BinaryBuilder infrastructure for native compilation across platforms.

@JuliaTagBot JuliaTagBot removed the AutoMerge: last run blocked by comment PR blocked by one or more comments lacking the string [noblock]. label Mar 19, 2025
@goerz
Copy link
Member

goerz commented Mar 19, 2025

once I finish writing the README file.

I would generally ask that the README file is complete before the registration goes through. The general point, is to have a description of the package's purpose and a small usage example in (or linked from) the README. An important part of packages in General is that any potential user can figure out what the package is about and how to get started with using it. That is really difficult when there is a lack of documentation.

In the longer term, I definitely recommend setting up a Documenter-based documentation. Before a v1.0 release, or for smaller packages that can be effectively described entirely with their README, that's not a requirement, though.

I also wanted to make a general comment on the name: I haven't been able to find out very much about the original KRAKEN, specifically whether it is an acronym or not. It seems like it is not, but that the uppercase spelling is a remnant of Fortran code using uppercase formatting back in the day. If it was an acronym (and widely recognized as such), KRAKEN.jl in #127169 would have actually been appropriate. Just for future reference, the bot-rule against all uppercase letters is not a hard rule. Changing a package name just to make a bot happy is usually not a good idea. Alas, in this case, Kraken seems like the appropriate name, so let's stick to that.

That being said, a search shows the "Kraken" is a heavily overloaded name in software projects. At least, there's a popular OCR project, a cryptocurrency exchange, an image optimizer and possibly more. That makes me a little uneasy about monopolizing the name in Kraken.jl. My recommendation would probably be for AcousticKraken.jl. I leave this up to your best judgment, though. Ultimately, there is a bit of a "first come, first serve" policy. That doesn't mean it's not a good idea to disambiguate package names as much as possible, since the General registry has to serve a very wide audience.

When you renamed from KRAKEN to Kraken, it seems like the uppercase spelling is still in a lot of places, like the README and the tests. That also means the tests aren't currently being run. I strongly recommend setting up automated tests before registering the package.

@goerz
Copy link
Member

goerz commented Mar 19, 2025

Oh, I also just noticed you are already at a version > 1.0. If you intend to follow semantic versioning, that means your package should have a stable and fully documented API, and that you track breaking and non-breaking changes. In my opinion, the latter requires automated tests with decent coverage. So I would very much recommend that you polish up the package a bit more with full documentation and testing before continuing with the registration.

@JuliaTagBot JuliaTagBot added the AutoMerge: last run blocked by comment PR blocked by one or more comments lacking the string [noblock]. label Mar 19, 2025
@goerz
Copy link
Member

goerz commented Mar 19, 2025

I noticed that you have so and dll files checked into your repo. This may make things tricky to support different architectures and OSs. You may want to consider using BinaryBuilder infrastructure for native compilation across platforms.

I'm not 100% sure if there's a hard rule against compilation artifacts in the src folder, but I think there is (if I'm not making up memories of earlier discussions on this issue). We should probably add automated checks for binary files in src. In any case, BinaryBuilder is definitely the correct way to handle this.

So that's probably more of a blocker for the registration of this package than the naming or the state of documentation/testing.

@vardister
Copy link

vardister commented Mar 19, 2025

[noblock]

@vardister Would it be possible to implement the model API in UnderwaterAcoustics.jl in Kraken.jl? That way it can become a model that can easily swapped in by anyone who already uses UnderwaterAcoustics.jl? We have some changes to the API in the upcoming version, so I could work with you on this.

I noticed that you have so and dll files checked into your repo. This may make things tricky to support different architectures and OSs. You may want to consider using BinaryBuilder infrastructure for native compilation across platforms.

@mchitre It's absolutely possible! I would be happy to be working with you on that. What's the best channel of communication for you?

And thanks for the suggestion about the binary files. I thought I would get by with a Make file, but BinaryBuilder is definitely the way to go.

@vardister
Copy link

vardister commented Mar 19, 2025

[noblock]

Oh, I also just noticed you are already at a version > 1.0. If you intend to follow semantic versioning, that means your package should have a stable and fully documented API, and that you track breaking and non-breaking changes. In my opinion, the latter requires automated tests with decent coverage. So I would very much recommend that you polish up the package a bit more with full documentation and testing before continuing with the registration.

You are correct. I will get it back to version 1.0 and start from there. I will polish a test set for this as well.

I would generally ask that the README file is complete before the registration goes through. The general point, is to have a description of the package's purpose and a small usage example in (or linked from) the README. An important part of packages in General is that any potential user can figure out what the package is about and how to get started with using it. That is really difficult when there is a lack of documentation.

Good opportunity to work on my README file then!

I also wanted to make a general comment on the name: I haven't been able to find out very much about the original KRAKEN, specifically whether it is an acronym or not. It seems like it is not, but that the uppercase spelling is a remnant of Fortran code using uppercase formatting back in the day. If it was an acronym (and widely recognized as such), KRAKEN.jl in #127169 would have actually been appropriate. Just for future reference, the bot-rule against all uppercase letters is not a hard rule. Changing a package name just to make a bot happy is usually not a good idea. Alas, in this case, Kraken seems like the appropriate name, so let's stick to that.

KRAKEN is just a very well known name for a program in the field of underwater acoustics, and it is referenced in dozens, if not hundreds, of papers in the field. My thinking goes that it would make it much easier for people in the field to recognize the program and understand its implementation, as it is quite similar to the original Fortran version. I will give it some thought though following your comments.

I'm not 100% sure if there's a hard rule against compilation artifacts in the src folder, but I think there is (if I'm not making up memories of earlier discussions on this issue). We should probably add automated checks for binary files in src. In any case, BinaryBuilder is definitely the correct way to handle this.

So that's probably more of a blocker for the registration of this package than the naming or the state of documentation/testing.

I deleted the binary files. I do have a Make file ready to go. Will try and use it with BinaryBuilder.

@goerz Generally, wanted to say thanks for for all the helpful comments!

@mchitre
Copy link

mchitre commented Mar 20, 2025

[noblock]

@vardister I think the suggestion was to go to a pre-release version 0.x.y if you feel that the API may change in the initial days of the package. To follow semantic versioning, once you have a 1.x.y version, any breaking changes (e.g. API changes that will force users to change their code) will force you to have a major version change to 2.x.y.

@goerz
Copy link
Member

goerz commented Mar 20, 2025

I think the suggestion was to go to a pre-release version 0.x.y

That's one possibility, the other being to go to 1.0 (or keep 1.2), but bringing the documentation and tests into shape, for that label to be justified.

Personally, I would usually make the initial public release <1.0. You can still go from, e.g., v0.1 to v1.0 without substantive changes later on, if the original API proves to be useful. However, for a package like this, where the API is determined by the original KRAKEN project, there might be less room for the "experimental" pre-1.0 stage of development. An initial public version 1.0 would then be fine, provided it can live up to the expectations set by semantic versioning.

KRAKEN is just a very well known name for a program in the field of underwater acoustics, and it is referenced in dozens, if not hundreds, of papers in the field.

Just to clarify this further: Different languages have different naming conventions. Especially with compiled languages like Fortran/C++, there's never been a central authority, and names tend towards "short" and also towards acronyms. This is very much not the case with Julia, where the community expectation is mostly for LongDescriptiveName (unless there's a witty short name like Makie or something like that). Acronyms in particular are very frowned upon. I recommend that when people switch from some other language to Julia, they adapt to Julia's conventions instead of bringing over conventions from the other language.

There are exceptions to the naming guidelines for wrappers, or potentially for translations as well. The automatic rule again all-cap names is mainly to protect against random acronyms. At least in my opinion, it should not cover pronounceable acronyms. But still, it doesn't seem like the original KRAKEN is actually an acronym, so I think the more Julian variation of that name, Kraken (or AcousticKraken), is preferable and still has plenty of name recognitions to potential users familiar with the traditional KRAKEN.

@vardister
Copy link

I think I will strive towards getting the version down below 1.0.0 as I won't have nearly enough time to get everything in order soon.

@goerz Should I go with getting a new issue opened with the registration bot or just commit the new version numbering?

@mchitre Would you have a good suggestion for a good package name? AcousticKraken does sound good though.

@mchitre
Copy link

mchitre commented Mar 24, 2025

[noblock]

I personally like just Kraken.jl, since it is not likely to be confused with anything else and directly reflects the name of the original model. But AcousticKraken.jl or KrakenModes.jl or KrakenAcousticModes.jl could work too.

@goerz
Copy link
Member

goerz commented Mar 24, 2025

Should I go with getting a new issue opened with the registration bot or just commit the new version numbering?

If you make any changes, you just retrigger the registration the same way you did the original registration. Personally, I just comment @Registerator register on the commit that I want to be tagged as the release. I never really understood the process of doing this via issues, but presumably that works as well.

In any case, if you change the name of the package or the version number, this will automatically create a new registration PR here (and we would close this PR in favor of the new one). For other things, like updating the documentation, or hypothetically fixing many of the issues that the registration bot flags (like invalid compat bounds), retriggering the registration would update an existing PR.

[noblock]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants