-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
[REVIEW]: VTUFileHandler: A VTU library in the Julia language that implements an algebra for basic mathematical operations on VTU data #4300
Comments
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks. For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
|
Wordcount for |
This is the reviewing thread. First, please type
to generate checklist. In this checklist there will be 20 check items for each reviewer. Whenever you complete the corresponding item, you can check them on. The reviewing process is interactive so you can always interact with the author(s) and the editor. You can discuss issues here as well as in target the repository by opening new issues. If you open a new issue in the target repository, please mention them here so we can keep tracking what is going on outside the main thread. Thank you in advance. |
@baxmittens - at a first glance, it seems a citation for Julia is missing. could you please consider adding it? @dmbates, @mkitti - could you please create your checklist by typing the editorialbot command given above? Thank you all in advance. |
Review checklist for @mkittiConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Review checklist for @dmbatesConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
The authors introduce VTUFileHandler.jl, a Julia language package for handling VTK unstructured grid data. The package also provides a number of mathematical operators reminiscent of an algebraic field over addition and multiplication.
Does the author intend to register one or both packages? Given the the XMLParser module is bespoke, is it really meant to be an independent package? Could it exist as a module within VTUFileHandler.jl?
julia> nbytes(UInt8)
1
julia> sizeof(UInt32)
4
julia> sizeof(UInt64)
8
julia> sizeof(Float64)
8
julia> sizeof(Int32)
4
julia> sizeof(Int64)
8
julia> sizeof(UInt8)
1 Could the definition To be continued... |
Gladly did so. |
@editorialbot check references |
|
@editorialbot generate pdf |
@mkitti - thank you for your comments. I would like to answer to them
Fixed the typo.
Not at this point but maybe later. I elaborate on this below
I use it in other projects of mine too. For example for some simple html and OpenGeoSys input file manipulations. Thinking in term of functions, XMLParser.jl should be a stand-alone package since it has a distinctive function. I'm no big fan of mixing too many diffierent functionalities in one project since it complicates the expandability and the maintainbility. And finally, even VTUFileHandler.jl is only part of a bigger project. As I wrote earlier (in Prereview), I implemented an adaptive sparse-grid colloction method (see sparse grid theory paper) which uses the VTUFileHandler and a broader stochastic / hpc framework which uses the adaptive sparse-grid. I could throw this all in one package but I thought it would be much better to distribute this big project in smaller packages according to their functionalities. I plan on publishing two consecutive joss-papers describing the sparse-grid and the stochastic-hpc-framework
Before starting this project, I looked into both WriteVTK.jl and ReadVTK.jl and at that point both package were not really a good fit for what I was planning on doing. But as things progress I would definetly consider rebasing my stuff onto their projects.
As mentioned in the paper, if operators should be applied the objects have to share the same topology. If they do, all operators act point-wise on each corresponding pair of e.g. nodal coefficients. The operators itself keep their properties as they perform their action on a collection of scalars. Each nodal coefficient
Thank you. Changed all nbytes to sizeof. That makes a lot more sense... Sorry for the long answer. |
Looks like there are lots of "my needs" instead of "community's needs", the software can be refactored in lights of the reviews. The dependency XMLParser as declared in the Project.toml is not a standard Julia package - I am sharing the same thoughts with the reviewer. Altought it is not strictly needed, we always encourage authors to submit their packages so it fits well with the eco-system of the language and the environments. At the end of the story, the software should solve a generalized problem with an easy and painless installation process. |
Agree, this was a project that originally was created only in my needs and I was asked to open-source it because other people wanted to use it. But I do think it solves a generalized problem quite handy. |
Regarding package registration, I do not see how dependence on the other packages or lack there of would prevent package registration. This can always be modified in subsequent vesions of the package. VTUFileHandler.jl is either a reusable library or a terminal application.
Per the review criteria on https://joss.readthedocs.io/en/latest/review_criteria.html#installation-instructions:
Julia has a mature software packaging system and for publication VTUFileHandler.jl should take full advantage of it. I will let the author decide between the two options above, but one of them must be chosen as a requirement of publication. The choice will also affect how to integrate the XMLParser module. See https://github.com/JuliaRegistries/General#should-i-register-my-package |
Thank you for this comment. Please clarify the above points in the paper, clearly delineating how this library implements an algebra such as a field. If you feel there is an alternative algebra, please state what it is. Specifically there are a couple missing parts I have identified so far:
|
It would be good to document this in the README for XMLParser. |
Sorry but I disagree, here. Via import Pkg
Pkg.add(url="https://github.com/baxmittens/XMLParser.jl.git")
Pkg.add(url="https://github.com/baxmittens/VTUFileHandler.jl.git") the package is registered in the julia package manager. it will be automatically updated if Pkg.update() is called and a new version is available. All dependencies are listed in the
Edit: While reading further into the topic, I don't know if the above is right. It seems like the Manifest.toml would be added automatically, if the I would register the Project in the Julia package manager. But still, adding a Manifest.toml manually to the repo doesn't make sense, right? |
Thank you for the suggestion. Added it in the readme. |
Let V=(VTUFile,+,*) be a field and A \subseteq R^n a vector space over V. Then V is an algebra if for all x,y,z \in A and a,b \in V the following holds:
which surely applies but I don't really think this should be included in the paper or should it?
Added those, thank you. |
The language of the title is very specific. So yes, I think the contents of the a article should support the statement in the title. |
@editorialbot generate pdf |
@baxmittens could you please fix those items in paper? please have a full read and correct other typos if exist. |
@editorialbot generate pdf |
@baxmittens - it seems okay now. Could you please continue with the following steps that I've mentioned above? |
@jbytecode - I'm just about finishing...sorry for taking that long |
@baxmittens - no worries, thank you. |
|
@editorialbot set v0.2.0 as version |
Done! version is now v0.2.0 |
@editorialbot set 10.5281/zenodo.6576155 as archive |
Done! Archive is now 10.5281/zenodo.6576155 |
@editorialbot recommend-accept |
|
|
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#3226 If the paper PDF and the deposit XML files look good in openjournals/joss-papers#3226, then you can now move forward with accepting the submission by compiling again with the command |
@baxmittens thank you for the submission, @mkitti and @dmbates thank you for your valuable reviews and consuming your time for JOSS. One of the @openjournals/joss-eics will have the final decision. Thank you! |
@jbytecode @mkitti @dmbates - thank you for reviewing my first joss article. |
@editorialbot accept |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
@dmbates, @mkitti – many thanks for your reviews here and to @jbytecode for editing this submission! JOSS relies upon the volunteer effort of people like you and we simply wouldn't be able to do this without you ✨ @baxmittens – your paper is now accepted and published in JOSS ⚡🚀💥 |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: We need your help! The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:
|
Submitting author: @baxmittens (maximilian bittens)
Repository: https://github.com/baxmittens/VTUFileHandler
Branch with paper.md (empty if default branch):
Version: v0.2.0
Editor: @jbytecode
Reviewers: @dmbates, @mkitti
Archive: 10.5281/zenodo.6576155
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@dmbates & @mkitti, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @jbytecode know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Checklists
📝 Checklist for @mkitti
📝 Checklist for @dmbates
The text was updated successfully, but these errors were encountered: