R: Re: R: Re: kde review kartesio
Sure, I thought it was already back to playground-edu. Luca Tringali Messaggio originale Da: aa...@kde.org Data: 27/10/2013 19.03 A: LucaTringalitringalinv...@libero.it, kde-core-devel@kde.org Ogg: Re: R: Re: kde review kartesio El Diumenge, 27 d'octubre de 2013, a les 18:48:21, LucaTringali va escriure: Hi Albert, I'm working on the points but, since I'm kinda busy with my work, the code I have written for now is still not stable. I think it'll be ready in the first week of January 2014. If you study something, at high school or university, which includes experiments (chemistry, phisycs, biology, engineering, etc...) you need a best fit calculator. So, philosophically, it is something very similar to Kalzium Calculator. Ok, I see your point. Maybe we should move it back from kdereview to playgound-edu? It's not the idea that stuff stays in kdereview for a long time. Cheers, Albert Luca Tringali Messaggio originale Da: aa...@kde.org Data: 27/10/2013 18.30 A: kde-core-devel@kde.org Ogg: Re: kde review kartesio El Dijous, 9 de maig de 2013, a les 18:06:16, LucaTringali va escriure: Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali Hi Luca, have you worked on addressing the points raised in the comments you were made? Also I am wondering if kdeedu is really the place for this app, I understand it's a very useful application for science/laboratory situations, but is that really something a student would use? Cheers, Albert
R: Re: kde review kartesio
Hi Albert, I'm working on the points but, since I'm kinda busy with my work, the code I have written for now is still not stable. I think it'll be ready in the first week of January 2014. If you study something, at high school or university, which includes experiments (chemistry, phisycs, biology, engineering, etc...) you need a best fit calculator. So, philosophically, it is something very similar to Kalzium Calculator. Luca Tringali Messaggio originale Da: aa...@kde.org Data: 27/10/2013 18.30 A: kde-core-devel@kde.org Ogg: Re: kde review kartesio El Dijous, 9 de maig de 2013, a les 18:06:16, LucaTringali va escriure: Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali Hi Luca, have you worked on addressing the points raised in the comments you were made? Also I am wondering if kdeedu is really the place for this app, I understand it's a very useful application for science/laboratory situations, but is that really something a student would use? Cheers, Albert
Re: kde review kartesio
написане Sun, 27 Oct 2013 19:30:56 +0200, Albert Astals Cid aa...@kde.org: El Dijous, 9 de maig de 2013, a les 18:06:16, LucaTringali va escriure: Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali Hi Luca, have you worked on addressing the points raised in the comments you were made? Also I am wondering if kdeedu is really the place for this app, I understand it's a very useful application for science/laboratory situations, but is that really something a student would use? Cheers, Albert Hi, Just a point of view. It is somewhat fit kdeedu ideally. At least in the same order of magnitude as Rocs or Cantor. There are many other professional tools for curves fitting (Scilab, Octave for various data fitting algorithms, RKWard and PSPP for regressions). Kartesio is very good to acknowledge students with the basics of some modern data fitting algorithms. If you think is does not, can the module for kde-science be created for such applications (Kartesio, Rocs, KST, Cantor, Cirkuit, KStars (its latest improvements are largely for professional and astronomy hobbyists, not for students ;) ) maybe RKWard if its developers ever decide to join)? Just my 2 cents. Best regards, Yuri
Re: Re: R: Re: kde review kartesio
On Mon, Oct 28, 2013 at 7:54 AM, LucaTringali tringalinv...@libero.itwrote: Sure, I thought it was already back to playground-edu. Nope - you needed to ask sysadmin. In any case, I have now moved it to playground/edu. Luca Tringali Regards, Ben Cooksley KDE Sysadmin Messaggio originale Da: aa...@kde.org Data: 27/10/2013 19.03 A: LucaTringalitringalinv...@libero.it, kde-core-devel@kde.org Ogg: Re: R: Re: kde review kartesio El Diumenge, 27 d'octubre de 2013, a les 18:48:21, LucaTringali va escriure: Hi Albert, I'm working on the points but, since I'm kinda busy with my work, the code I have written for now is still not stable. I think it'll be ready in the first week of January 2014. If you study something, at high school or university, which includes experiments (chemistry, phisycs, biology, engineering, etc...) you need a best fit calculator. So, philosophically, it is something very similar to Kalzium Calculator. Ok, I see your point. Maybe we should move it back from kdereview to playgound-edu? It's not the idea that stuff stays in kdereview for a long time. Cheers, Albert Luca Tringali Messaggio originale Da: aa...@kde.org Data: 27/10/2013 18.30 A: kde-core-devel@kde.org Ogg: Re: kde review kartesio El Dijous, 9 de maig de 2013, a les 18:06:16, LucaTringali va escriure: Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali Hi Luca, have you worked on addressing the points raised in the comments you were made? Also I am wondering if kdeedu is really the place for this app, I understand it's a very useful application for science/laboratory situations, but is that really something a student would use? Cheers, Albert
Re: kde review kartesio
El Dijous, 9 de maig de 2013, a les 18:06:16, LucaTringali va escriure: Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali Hi Luca, have you worked on addressing the points raised in the comments you were made? Also I am wondering if kdeedu is really the place for this app, I understand it's a very useful application for science/laboratory situations, but is that really something a student would use? Cheers, Albert
Re: R: Re: kde review kartesio
El Diumenge, 27 d'octubre de 2013, a les 18:48:21, LucaTringali va escriure: Hi Albert, I'm working on the points but, since I'm kinda busy with my work, the code I have written for now is still not stable. I think it'll be ready in the first week of January 2014. If you study something, at high school or university, which includes experiments (chemistry, phisycs, biology, engineering, etc...) you need a best fit calculator. So, philosophically, it is something very similar to Kalzium Calculator. Ok, I see your point. Maybe we should move it back from kdereview to playgound-edu? It's not the idea that stuff stays in kdereview for a long time. Cheers, Albert Luca Tringali Messaggio originale Da: aa...@kde.org Data: 27/10/2013 18.30 A: kde-core-devel@kde.org Ogg: Re: kde review kartesio El Dijous, 9 de maig de 2013, a les 18:06:16, LucaTringali va escriure: Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali Hi Luca, have you worked on addressing the points raised in the comments you were made? Also I am wondering if kdeedu is really the place for this app, I understand it's a very useful application for science/laboratory situations, but is that really something a student would use? Cheers, Albert
R: Re: R: Re: kde review kartesio
Hi, no, build.sh is not needed. Since I'm kinda lazy, I prepared a shell script to run the following commands: mkdir build cd build sudo make uninstall make clean rm CMakeCache.txt cmake .. -DCMAKE_INSTALL_PREFIX=`kde4-config --prefix` make sudo make install so I don't have to type all those every time I want to build Kartesio again. I'm not sure why you get that error, mainlybecause I don't know which is the instruction that gives that problem since I changed a lot the code in these hours. Try to download the latest git version and build it, so I will know exactly where the problem is. Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 11/05/2013 12.04 A: LucaTringalitringalinv...@libero.it Cc: kde-core-devel@kde.org Ogg: Re: R: Re: kde review kartesio Hi, Hi, actually I have not prepared any binary package. Anyway, you can install Kartesio downloading the source code from the git repo (https://projects.kde. org/projects/kdereview/kartesio), installing the library libzorbaneural (https: //www.gitorious.org/zorbaneural/zorbaneural/trees/master/binary- packages/libzorbaneural-0.1), and running the build.sh script you find in the Kartesio root folder. Why is this build.sh script needed? Is it because I did not run it that I got my build error? Also, you should have installed the program maxima (just the program, dev libraries are not needed) to have Kartesio fully working. If there are some troubles in building Kartesio, just ask me. I have this error: /home/kde-devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors Best regards, Anne-Marie
R: Re: kde review kartesio
Yes, I also think adding new features now is not a good idea in this moment.
This is the reason why the new version of Kartesio I uploaded to git (about 45
minutes ago) contains basically all the corrctions you suggested, but no new
features.
Luca Tringali
Messaggio originale
Da: annemarie.mahf...@free.fr
Data: 11/05/2013 12.15
A: LucaTringalitringalinv...@libero.it
Cc: kde-core-devel@kde.org
Ogg: Re: kde review kartesio
Hi,
In general, you're mixing a lot of plain C / stdlib stuff into Qt
code. Is there a reason for that? For example, in
calculations.cpp:148
you take text from a text field, convert it to a byte array, convert
it to a char* and then pass it to a function. Why not just pass the
QString? You can iterate over a QString like
foreach ( const QChar c, myqstring ) { ... }
or also
for ( int i = 0; i myqstring.size(); i++ ) { ... }
if you like that better, and you can also index it like a char*, as
in
mystring[i+1] or so.
Yes, this is an heritage from the older version of Kartesio, that was
based
mainly on plain ANSI C++. Those mixing are just an hack to make
Kartesio work
immediately. If I'll have time, I will translate everything into
Qt, but
first of all I would like to apply other features.
You asked for an inclusion in KDE and we are reviewing Kartesio. There is
already a big amount of work to be done from the comments you got. I don't
think adding features now is a smart move, review is a phase where your program
should reach KDE standards. Using Qt libs wherever possible is the priority and
getting all the required fixes will make you busy enough.
Best regards,
Anne-Marie
Re: R: Re: R: Re: R: Re: kde review kartesio
El Dissabte, 11 de maig de 2013, a les 14:37:06, LucaTringali va escriure: Ooops: I thought I already corrected this problem, for some reason I did not upload to git this correction. On my system the build goes fine also with that line, but I decide to remove it because it can cause problems with other compiler configurations (on mine I just get a wrining, on other systems it is an error). I corrected the line, anyway just commenting it should be fine: this line is not needed, and it presence does not affect the running of the program. I read that you have commented this line, so you should now be able to run Kartesio. To try it, you can load the file parabola.kartesio which is on the git repo. Hi, I'm pretty sure people following this thread would appreciate if you did not top post Cheers, Albert Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 11/05/2013 13.08 A: LucaTringalitringalinv...@libero.it Cc: kde-core-devel@kde.org Ogg: Re: R: Re: R: Re: kde review kartesio Hi, so I don't have to type all those every time I want to build Kartesio again. I'm not sure why you get that error, mainlybecause I don't know which is the instruction that gives that problem since I changed a lot the code in these hours. Try to download the latest git version and build it, so I will know exactly where the problem is. commenting out line 465 in calculations.cpp makes it build (the line after the return). Anne-Marie
Re: R: Re: kde review kartesio
Hi, I think Kartesio is not ready to move: - the GUI is not so good - lack of tooltips - I am not happy with some strings and they lack context info - the standard C++ code is not so good either (this rm for example) - lack of testing I suggest you do a release first so we can test translations and you can get some users feedback and have time to move the code to Qt classes. This is my suggestion only, others may disagree Best regards, Anne-Marie
Re: R: Re: kde review kartesio
Hi, I'm sorry, but I have to agree with Anne-Marie. Cheers! Sven 2013/5/12 Anne-Marie Mahfouf annemarie.mahf...@free.fr: Hi, I think Kartesio is not ready to move: - the GUI is not so good - lack of tooltips - I am not happy with some strings and they lack context info - the standard C++ code is not so good either (this rm for example) - lack of testing I suggest you do a release first so we can test translations and you can get some users feedback and have time to move the code to Qt classes. This is my suggestion only, others may disagree Best regards, Anne-Marie
Re: R: Re: kde review kartesio
El Divendres, 10 de maig de 2013, a les 14:18:43, LucaTringali va escriure: Hello, libzorbaneural can be found here: https://www.gitorious.org/zorbaneural/zorbaneural/trees/master Here are also some packages for the stable version (deb and rpm): https://www.gitorious.org/zorbaneural/zorbaneural/trees/master/binary- packages/libzorbaneural-0.1 Once you have installed it, the build should go fine. The screenshot folder and the .pro file are not needed, if they are a problem I can remove them. If they are not needed, yes, I'd kill them. Cheers, Albert Talking about the system call in calculations, this is the reason why actuallt Kartesio works only on GNU/Linux systems. Why I did it? Because it was the easier way to do that. In the next release of Kartesio, this problem will be solved. I'm not very practical with translatable strings, so I excuse for the Message. sh: is there a wiki page to understand how to write a Message.sh file? I'm writing comments on variables in header files, in the next hours I'll publish them into git. Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 10/05/2013 13.58 A: LucaTringalitringalinv...@libero.it Cc: kde-core-devel@kde.org Ogg: Re: kde review kartesio Hi, A few primary remarks: - libzorbaneural is needed but my distro does not have anything with neural in it (OpenSuse 12.3) what repo do I need to add in order to get it? The libzorbaneural website should be added to the cmake file so people can find this and packagers can add it to their distros. - I see a screenshot folder and some .pro files that probably are not needed - some doxygen comments for the variables in the .h files would be appreciated, if anyone else wants to fix bugs it'll help a lot. - Kartesio does not build for me, I get /home/kde- devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors - I don't see a Messages.sh file to extract translatable strings. - I am not comfortable with the rm call line 181 in calculations.cpp = you can probably use more Qt classes here and in other parts of this file too. That's only a quick review as I couldn't run the app yet. Tomaz, as for the user base maybe we could start a module for advanced scientific tools? Best regards, Anne-Marie - Mail original - De: Tomaz Canabrava tcanabr...@kde.org À: Anne-Marie Mahfouf annemarie.mahf...@free.fr Cc: LucaTringali tringalinv...@libero.it, kde-core-devel@kde.org Envoyé: Vendredi 10 Mai 2013 12:28:54 Objet: Re: kde review kartesio Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
Re: Re: kde review kartesio
Yes, Kartesio is used to calculate fitting curves for experimental points: I, as a chemistry student, already used it for some laboratory reports. Actually, there is no other program like this in KDE: obiously you could obtain something similar with RKward, but this one is too much complex for fitting curves, and students usually do not like R. Kartesio does one thing, and does it simply and good. Caculating best fit curves with R is smilar to cross a stream with the Queen Mary.The most interesting feature of Kartesio is that it allows you to write manually the equation you want to use to fit the points (with other programs like LibreOffice Calc it's possible only to use 3 or 4 already implemented and generic functions). For example, I can choose to fit my points with y=a*sin(b*x) or with y=(9.342/x)+c. Luca Tringali Messaggio originale Da: tcanabr...@kde.org Data: 10/05/2013 12.28 A: Anne-Marie Mahfoufannemarie.mahf...@free.fr Cc: LucaTringalitringalinv...@libero.it, kde-core-devel@kde.org Ogg: Re: kde review kartesio Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
R: Re: kde review kartesio
Hello, libzorbaneural can be found here: https://www.gitorious.org/zorbaneural/zorbaneural/trees/master Here are also some packages for the stable version (deb and rpm): https://www.gitorious.org/zorbaneural/zorbaneural/trees/master/binary- packages/libzorbaneural-0.1 Once you have installed it, the build should go fine. The screenshot folder and the .pro file are not needed, if they are a problem I can remove them. Talking about the system call in calculations, this is the reason why actuallt Kartesio works only on GNU/Linux systems. Why I did it? Because it was the easier way to do that. In the next release of Kartesio, this problem will be solved. I'm not very practical with translatable strings, so I excuse for the Message. sh: is there a wiki page to understand how to write a Message.sh file? I'm writing comments on variables in header files, in the next hours I'll publish them into git. Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 10/05/2013 13.58 A: LucaTringalitringalinv...@libero.it Cc: kde-core-devel@kde.org Ogg: Re: kde review kartesio Hi, A few primary remarks: - libzorbaneural is needed but my distro does not have anything with neural in it (OpenSuse 12.3) what repo do I need to add in order to get it? The libzorbaneural website should be added to the cmake file so people can find this and packagers can add it to their distros. - I see a screenshot folder and some .pro files that probably are not needed - some doxygen comments for the variables in the .h files would be appreciated, if anyone else wants to fix bugs it'll help a lot. - Kartesio does not build for me, I get /home/kde- devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors - I don't see a Messages.sh file to extract translatable strings. - I am not comfortable with the rm call line 181 in calculations.cpp = you can probably use more Qt classes here and in other parts of this file too. That's only a quick review as I couldn't run the app yet. Tomaz, as for the user base maybe we could start a module for advanced scientific tools? Best regards, Anne-Marie - Mail original - De: Tomaz Canabrava tcanabr...@kde.org À: Anne-Marie Mahfouf annemarie.mahf...@free.fr Cc: LucaTringali tringalinv...@libero.it, kde-core-devel@kde.org Envoyé: Vendredi 10 Mai 2013 12:28:54 Objet: Re: kde review kartesio Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
Re: Re: kde review kartesio
Hi,
I'll answer point to point:
Messaggio originale
Hey!
A good thing, I think such a tool could be useful to me too (and I
know a lot of other people to whom it might be useful). Here's what I
noticed from a quick look (some has been said already I think):
* You probably shouldn't track the kdev4 file in the repository, same
goes for screenshots
Yes, those files are there just because I found them useful, but I can remove
them without any problem.
* zorbaneural is a very fancy dependency, it's not even in arch's AUR.
You should put the git URL into the cmake message.
I know, I wrote where to find it in Kartesio project overview but, since
almost everybody had some troubles with it, I will make this thing more clear.
* I put x**2 into the fit box and clicked Fit, and it crashed:
http://paste.kde.org/741026/
Yes, the correct way to express a power is ^. So you should write x^2. There
should be a check routine to avoid that a dangerous string like ** is used,
and I'm surely integrating this check in the next release.
* Did you think about laying out the UI around a splitter? On my
screen, the table takes most of the area and the plot is quite small,
and I can't change that...
This could be a good idea: another thing for the next release.
* It would be useful to be able to import data in some way, e.g. from
a CSV file. I don't see a way to get data into the program except
typing every number into the cells -- or is there another way? If
there is, could it be made more obvious eventually?
Actually there is not: I'm working on a new window for the next releases:
basically, there will be a button over the table, something like Edit datas.
This will open a new window in which it will be possible to import/export CSV,
sort X axis values, add other rows or deleting some...
* What does the code in calculations.cpp:117 do? It looks quite
curious. Isn't there a more elegant solution (it looks a bit like a
QChar::isLetter() implementation)?
* calculations.cpp:505 and 584 the same code like in 117 again? It's
weird enough to have that stuff once, but copied multiple times is bad
imho ;)
No, at least not only. Originally, line 117 and 118 were collapsed into one
single if instruction, it slip it in two because it was too long. This line
checks if the current char (which is a C++ char an not a QChar) is permitted or
not. Permitted characters ar letters, numbers, and some other simbols (for
example +, (, etc..).
Could this instruction be shorter and more elagant? Probably. But it works,
and actually I think it could stay as it is.
* Your code uses mixed tab- and space indent (sometimes it uses tabs,
sometimes spaces for no apparent reason). Most KDE apps use only
spaces, you might consider if you want to do that too. Sometimes, the
indent is even missing completely; you should indent one level after
each opening curly parenthesis.
* Same goes for the whole formatting of the code, it's pretty
inconsistent. For example, look at the spaces around operators or so.
I know it, I'll try to make the code more readable, but I do not have so much
time so usually I prefer to dedicate my time to new features or corrections
instead of making them prettier.
* Instead of writing to /tmp/kartesiotmp.txt you should probably use
QTempFile. That will also take care of the deleting the temp file when
it gets deallocated so you don't need to exec (scary and
platform-dependent) rm commands.
I'm already working with QTempFile for the next release of Kartesio.
* calculations.cpp:277 this makes no sense, there's a statement behind
a return
Ooops: I thought I already removed it.
In general, you're mixing a lot of plain C / stdlib stuff into Qt
code. Is there a reason for that? For example, in calculations.cpp:148
you take text from a text field, convert it to a byte array, convert
it to a char* and then pass it to a function. Why not just pass the
QString? You can iterate over a QString like
foreach ( const QChar c, myqstring ) { ... }
or also
for ( int i = 0; i myqstring.size(); i++ ) { ... }
if you like that better, and you can also index it like a char*, as in
mystring[i+1] or so.
Yes, this is an heritage from the older version of Kartesio, that was based
mainly on plain ANSI C++. Those mixing are just an hack to make Kartesio work
immediately. If I'll have time, I will translate everything into Qt, but
first of all I would like to apply other features.
Also, nothing in your code is const and everything is public, although
almost everything could be const and private, but I won't get started
on that now ;)
This is not meant as a list of what you must fix, it's just my two cents.
Thank you, it's always nice to get some suggestions.
Cheers!
Sven
2013/5/10 Tomaz Canabrava tcanabr...@kde.org:
Annma, I find that proposal *very* good.
I'm a bit distant of KDE programming - I know - because my day job is
making
me work 12h+ creating scientific tools.
( actually - one of the tools that I
Re: R: Re: kde review kartesio
Hi, Hi, actually I have not prepared any binary package. Anyway, you can install Kartesio downloading the source code from the git repo (https://projects.kde. org/projects/kdereview/kartesio), installing the library libzorbaneural (https: //www.gitorious.org/zorbaneural/zorbaneural/trees/master/binary- packages/libzorbaneural-0.1), and running the build.sh script you find in the Kartesio root folder. Why is this build.sh script needed? Is it because I did not run it that I got my build error? Also, you should have installed the program maxima (just the program, dev libraries are not needed) to have Kartesio fully working. If there are some troubles in building Kartesio, just ask me. I have this error: /home/kde-devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors Best regards, Anne-Marie
R: Re: kde review kartesio
Hello everybody, just wanted to tell you that I made some fixes to the code, based on your suggestions: *comments in header files *deleted some unseful string *check routine to avoid that a dangerous string like ** or similar is used for the function *add where to download zorbaneural in cmake module *in neural network algortihm, check if all the points are between 0 and 1 *check if the maxima report is empty before showing it *added file messages.sh I also cleaned it up a little, to make it more readable. Luca Tringali Messaggio originale Da: tringalinv...@libero.it Data: 10/05/2013 14.18 A: kde-core-devel@kde.org, annemarie.mahf...@free.fr Ogg: R: Re: kde review kartesio Hello, libzorbaneural can be found here: https://www.gitorious.org/zorbaneural/zorbaneural/trees/master Here are also some packages for the stable version (deb and rpm): https://www.gitorious.org/zorbaneural/zorbaneural/trees/master/binary- packages/libzorbaneural-0.1 Once you have installed it, the build should go fine. The screenshot folder and the .pro file are not needed, if they are a problem I can remove them. Talking about the system call in calculations, this is the reason why actuallt Kartesio works only on GNU/Linux systems. Why I did it? Because it was the easier way to do that. In the next release of Kartesio, this problem will be solved. I'm not very practical with translatable strings, so I excuse for the Message. sh: is there a wiki page to understand how to write a Message.sh file? I'm writing comments on variables in header files, in the next hours I'll publish them into git. Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 10/05/2013 13.58 A: LucaTringalitringalinv...@libero.it Cc: kde-core-devel@kde.org Ogg: Re: kde review kartesio Hi, A few primary remarks: - libzorbaneural is needed but my distro does not have anything with neural in it (OpenSuse 12.3) what repo do I need to add in order to get it? The libzorbaneural website should be added to the cmake file so people can find this and packagers can add it to their distros. - I see a screenshot folder and some .pro files that probably are not needed - some doxygen comments for the variables in the .h files would be appreciated, if anyone else wants to fix bugs it'll help a lot. - Kartesio does not build for me, I get /home/kde- devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors - I don't see a Messages.sh file to extract translatable strings. - I am not comfortable with the rm call line 181 in calculations.cpp = you can probably use more Qt classes here and in other parts of this file too. That's only a quick review as I couldn't run the app yet. Tomaz, as for the user base maybe we could start a module for advanced scientific tools? Best regards, Anne-Marie - Mail original - De: Tomaz Canabrava tcanabr...@kde.org À: Anne-Marie Mahfouf annemarie.mahf...@free.fr Cc: LucaTringali tringalinv...@libero.it, kde-core-devel@kde.org Envoyé: Vendredi 10 Mai 2013 12:28:54 Objet: Re: kde review kartesio Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
Re: kde review kartesio
Hi,
In general, you're mixing a lot of plain C / stdlib stuff into Qt
code. Is there a reason for that? For example, in
calculations.cpp:148
you take text from a text field, convert it to a byte array, convert
it to a char* and then pass it to a function. Why not just pass the
QString? You can iterate over a QString like
foreach ( const QChar c, myqstring ) { ... }
or also
for ( int i = 0; i myqstring.size(); i++ ) { ... }
if you like that better, and you can also index it like a char*, as
in
mystring[i+1] or so.
Yes, this is an heritage from the older version of Kartesio, that was
based
mainly on plain ANSI C++. Those mixing are just an hack to make
Kartesio work
immediately. If I'll have time, I will translate everything into
Qt, but
first of all I would like to apply other features.
You asked for an inclusion in KDE and we are reviewing Kartesio. There is
already a big amount of work to be done from the comments you got. I don't
think adding features now is a smart move, review is a phase where your program
should reach KDE standards. Using Qt libs wherever possible is the priority and
getting all the required fixes will make you busy enough.
Best regards,
Anne-Marie
Re: R: Re: R: Re: kde review kartesio
Hi, so I don't have to type all those every time I want to build Kartesio again. I'm not sure why you get that error, mainlybecause I don't know which is the instruction that gives that problem since I changed a lot the code in these hours. Try to download the latest git version and build it, so I will know exactly where the problem is. commenting out line 465 in calculations.cpp makes it build (the line after the return). Anne-Marie
Re: kde review kartesio
Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
Re: kde review kartesio
Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
Re: kde review kartesio
Hi, A few primary remarks: - libzorbaneural is needed but my distro does not have anything with neural in it (OpenSuse 12.3) what repo do I need to add in order to get it? The libzorbaneural website should be added to the cmake file so people can find this and packagers can add it to their distros. - I see a screenshot folder and some .pro files that probably are not needed - some doxygen comments for the variables in the .h files would be appreciated, if anyone else wants to fix bugs it'll help a lot. - Kartesio does not build for me, I get /home/kde-devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors - I don't see a Messages.sh file to extract translatable strings. - I am not comfortable with the rm call line 181 in calculations.cpp = you can probably use more Qt classes here and in other parts of this file too. That's only a quick review as I couldn't run the app yet. Tomaz, as for the user base maybe we could start a module for advanced scientific tools? Best regards, Anne-Marie - Mail original - De: Tomaz Canabrava tcanabr...@kde.org À: Anne-Marie Mahfouf annemarie.mahf...@free.fr Cc: LucaTringali tringalinv...@libero.it, kde-core-devel@kde.org Envoyé: Vendredi 10 Mai 2013 12:28:54 Objet: Re: kde review kartesio Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
Re: kde review kartesio
The app sounds awesome. From the application life cycle page you linked: When you have made one of more releases and want to continue to develop it, the term 'playground' does no longer apply to you. That is the right time to move out of here There are no releases on download.kde.org under unstable. Have you made these releases elsewhere? If so can you provide a link. Thanks David Edmundson
Re: kde review kartesio
Annma, I find that proposal *very* good. I'm a bit distant of KDE programming - I know - because my day job is making me work 12h+ creating scientific tools. ( actually - one of the tools that I created here was a... Solver, to fit curves on points... ) Tomaz 2013/5/10 David Edmundson da...@davidedmundson.co.uk The app sounds awesome. From the application life cycle page you linked: When you have made one of more releases and want to continue to develop it, the term 'playground' does no longer apply to you. That is the right time to move out of here There are no releases on download.kde.org under unstable. Have you made these releases elsewhere? If so can you provide a link. Thanks David Edmundson
Re: kde review kartesio
Hey!
A good thing, I think such a tool could be useful to me too (and I
know a lot of other people to whom it might be useful). Here's what I
noticed from a quick look (some has been said already I think):
* You probably shouldn't track the kdev4 file in the repository, same
goes for screenshots
* zorbaneural is a very fancy dependency, it's not even in arch's AUR.
You should put the git URL into the cmake message.
* I put x**2 into the fit box and clicked Fit, and it crashed:
http://paste.kde.org/741026/
* Did you think about laying out the UI around a splitter? On my
screen, the table takes most of the area and the plot is quite small,
and I can't change that...
* It would be useful to be able to import data in some way, e.g. from
a CSV file. I don't see a way to get data into the program except
typing every number into the cells -- or is there another way? If
there is, could it be made more obvious eventually?
* What does the code in calculations.cpp:117 do? It looks quite
curious. Isn't there a more elegant solution (it looks a bit like a
QChar::isLetter() implementation)?
* calculations.cpp:505 and 584 the same code like in 117 again? It's
weird enough to have that stuff once, but copied multiple times is bad
imho ;)
* Your code uses mixed tab- and space indent (sometimes it uses tabs,
sometimes spaces for no apparent reason). Most KDE apps use only
spaces, you might consider if you want to do that too. Sometimes, the
indent is even missing completely; you should indent one level after
each opening curly parenthesis.
* Same goes for the whole formatting of the code, it's pretty
inconsistent. For example, look at the spaces around operators or so.
* Instead of writing to /tmp/kartesiotmp.txt you should probably use
QTempFile. That will also take care of the deleting the temp file when
it gets deallocated so you don't need to exec (scary and
platform-dependent) rm commands.
* calculations.cpp:277 this makes no sense, there's a statement behind
a return
In general, you're mixing a lot of plain C / stdlib stuff into Qt
code. Is there a reason for that? For example, in calculations.cpp:148
you take text from a text field, convert it to a byte array, convert
it to a char* and then pass it to a function. Why not just pass the
QString? You can iterate over a QString like
foreach ( const QChar c, myqstring ) { ... }
or also
for ( int i = 0; i myqstring.size(); i++ ) { ... }
if you like that better, and you can also index it like a char*, as in
mystring[i+1] or so.
Also, nothing in your code is const and everything is public, although
almost everything could be const and private, but I won't get started
on that now ;)
This is not meant as a list of what you must fix, it's just my two cents.
Cheers!
Sven
2013/5/10 Tomaz Canabrava tcanabr...@kde.org:
Annma, I find that proposal *very* good.
I'm a bit distant of KDE programming - I know - because my day job is making
me work 12h+ creating scientific tools.
( actually - one of the tools that I created here was a... Solver, to fit
curves on points... )
Tomaz
2013/5/10 David Edmundson da...@davidedmundson.co.uk
The app sounds awesome.
From the application life cycle page you linked:
When you have made one of more releases and want to continue to develop
it, the term 'playground' does no longer apply to you. That is the right
time to move out of here
There are no releases on download.kde.org under unstable. Have you
made these releases elsewhere? If so can you provide a link.
Thanks
David Edmundson
Re: Re: Re: kde review kartesio
On Fri, May 10, 2013 at 3:38 PM, LucaTringali tringalinv...@libero.it wrote: The fact is that, before releasing the first binary package, I would like to be sure the code respects KDE guidelines. Otherwise, I would need to create a second package just to adjust the code for KDE. Luca Tringali Messaggio originale Da: da...@davidedmundson.co.uk Data: 10/05/2013 15.23 A: LucaTringalitringalinv...@libero.it Ogg: Re: Re: kde review kartesio On Fri, May 10, 2013 at 2:49 PM, LucaTringali tringalinv...@libero.it wrote: Hi, actually I have not prepared any binary package. If you have not reached a point of making a 0.1 release I would say it is not ready to be considered for extragear at this point in time. I have no objections to reviews before a release, that makes a lot of sense :) Hopefully those above help. I merely object to a move to extragear before making any releases, and so do the guidelines, which is how I interpreted your initial email. Regards David
Re: Re: kde review kartesio
Hi Luca, Yes, the correct way to express a power is ^. So you should write x^2. I figured that after it had crashed ;) There should be a check routine to avoid that a dangerous string like ** is used, and I'm surely integrating this check in the next release. In my opinion you should not release your application with such an obvious crash bug. It's not only this expression, the program crashes whenever there is anything wrong in that text field. That's not something you can defer to the next release. Actually there is not: I'm working on a new window for the next releases: basically, there will be a button over the table, something like Edit datas. This will open a new window in which it will be possible to import/export CSV, sort X axis values, add other rows or deleting some... I don't know if it makes sense, but you should have a look at the calligra sheets part (aka talk to someone who knows about it). Eventually that can be very useful for this purpose (unfortunately I don't know exactly how they work). Could this instruction be shorter and more elagant? Probably. But it works, and actually I think it could stay as it is. You could replace a 2000+ character boolean logic expression which lists all the letters in the alphabet by this: c.isLetter() || QString(+-*/^).contains(c) at least assuming you do the other thing I said and use QString instead of char*. This is a cleanup which is worth doing. I know it, I'll try to make the code more readable, but I do not have so much time so usually I prefer to dedicate my time to new features or corrections instead of making them prettier. Writing correctly formatted code is mostly a matter of setting up your editor correctly. Spend five minutes doing that ;) Greetings, Sven
