Adding save functionality and UI

[ashishsingh18]

Fix #22

[AbdulkadirA]

@ashishsingh18 Thanks. Worked as expected. Please make a single commit, because all the changes belong together.

git reset --soft 4afdb67da45c3ef9560b0a0e0426571f70b1dd2b
git add iSTAGING/dataio.py
git add iSTAGING/datamodel.py
git add iSTAGING/mainwindow.py
git commit -m "Add function to save data as "*.pkl.gz"
# Assuming your personal fork is called `origin` and you work on local branch `SaveFeature`
git push -f origin SaveFeature:SaveFeature

[ashishsingh18]

@ashishsingh18 Thanks. Worked as expected. Please make a single commit, because all the changes belong together.

git reset --soft 4afdb67da45c3ef9560b0a0e0426571f70b1dd2b
git add iSTAGING/dataio.py
git add iSTAGING/datamodel.py
git add iSTAGING/mainwindow.py
git commit -m "Add function to save data as "*.pkl.gz"
# Assuming your personal fork is called `origin` and you work on local branch `SaveFeature`
git push -f origin SaveFeature:SaveFeature

What is wrong with multiple commits in a PR? I do not think this is really required plus it is a wastage of my time. Each commit happened in a different file and is small and clear enough to follow.

[AbdulkadirA]

@ashishsingh18 Thanks. Worked as expected. Please make a single commit, because all the changes belong together.

git reset --soft 4afdb67da45c3ef9560b0a0e0426571f70b1dd2b
git add iSTAGING/dataio.py
git add iSTAGING/datamodel.py
git add iSTAGING/mainwindow.py
git commit -m "Add function to save data as "*.pkl.gz"
# Assuming your personal fork is called `origin` and you work on local branch `SaveFeature`
git push -f origin SaveFeature:SaveFeature

What is wrong with multiple commits in a PR? I do not think this is really required plus it is a wastage of my time. Each commit happened in a different file and is small and clear enough to follow.

There is nothing wrong with multiple commits. However, since the four commits implement a single functionality, it seems natural to me to commit them as a single commit. A commit makes sense if you may want to revert to it. Only the last is functional, so the others are obsolete in my opinion.

I don't see it as a waste of time, because you could have committed each commit as an amendment to the previous one which would even be faster, because you would not even need to write a commit message.

[ashishsingh18]

@ashishsingh18 Thanks. Worked as expected. Please make a single commit, because all the changes belong together.

git reset --soft 4afdb67da45c3ef9560b0a0e0426571f70b1dd2b
git add iSTAGING/dataio.py
git add iSTAGING/datamodel.py
git add iSTAGING/mainwindow.py
git commit -m "Add function to save data as "*.pkl.gz"
# Assuming your personal fork is called `origin` and you work on local branch `SaveFeature`
git push -f origin SaveFeature:SaveFeature

What is wrong with multiple commits in a PR? I do not think this is really required plus it is a wastage of my time. Each commit happened in a different file and is small and clear enough to follow.

There is nothing wrong with multiple commits. However, since the four commits implement a single functionality, it seems natural to me to commit them as a single commit. A commit makes sense if you may want to revert to it. Only the last is functional, so the others are obsolete in my opinion.

I don't see it as a waste of time, because you could have committed each commit as an amendment to the previous one which would even be faster, because you would not even need to write a commit message.

Frequent and smaller commits are always preferable than a single monolithic commit spanning multiple files. I disagree that only the last commit here was functional; each of these are necessary and not obsolete and just the last commit by itself wouldn't have worked. These are not even amendments as each serves different purpose. We may have different working style but that doesn't mean that this approach is incorrect and I should change this. The code is doing what it is supposed to do and scrutinizing that is where we should be putting our energies on.

[AbdulkadirA]

@ashishsingh18 Thanks. Worked as expected. Please make a single commit, because all the changes belong together.

git reset --soft 4afdb67da45c3ef9560b0a0e0426571f70b1dd2b
git add iSTAGING/dataio.py
git add iSTAGING/datamodel.py
git add iSTAGING/mainwindow.py
git commit -m "Add function to save data as "*.pkl.gz"
# Assuming your personal fork is called `origin` and you work on local branch `SaveFeature`
git push -f origin SaveFeature:SaveFeature

What is wrong with multiple commits in a PR? I do not think this is really required plus it is a wastage of my time. Each commit happened in a different file and is small and clear enough to follow.

There is nothing wrong with multiple commits. However, since the four commits implement a single functionality, it seems natural to me to commit them as a single commit. A commit makes sense if you may want to revert to it. Only the last is functional, so the others are obsolete in my opinion.
I don't see it as a waste of time, because you could have committed each commit as an amendment to the previous one which would even be faster, because you would not even need to write a commit message.

Frequent and smaller commits are always preferable than a single monolithic commit spanning multiple files. I disagree that only the last commit here was functional; each of these are necessary and not obsolete and just the last commit by itself wouldn't have worked. These are not even amendments as each serves different purpose. We may have different working style but that doesn't mean that this approach is incorrect and I should change this. The code is doing what it is supposed to do and scrutinizing that is where we should be putting our energies on.

I See your point, and, again, I am not saying it is wrong. By amendment I mean it in Git terminology, not that a line of code was changed. I agree that frequent commits are useful. I often make dozens of commits until I complete the PR. However, these commits are not all to stay. Eventually I organize them in digestible packages, often a single commit. Consider this commit history:

Add function to compute mean
Add option for harmonic mean
Fix typo in help text
Catch error if input is not numpy array

While it makes sense to have these commits during development, I don't see a reason to have all of them in the PR, especially the Fix typo in help text. That fix should be included in Add function to compute mean. Would you agree that at least Fix typo in help text is obsolete?

Note, I am fine with keeping four commits. Still I think one is better. It's your call. The code itself is nice and clean.

[ashishsingh18]

@ashishsingh18 Thanks. Worked as expected. Please make a single commit, because all the changes belong together.

git reset --soft 4afdb67da45c3ef9560b0a0e0426571f70b1dd2b
git add iSTAGING/dataio.py
git add iSTAGING/datamodel.py
git add iSTAGING/mainwindow.py
git commit -m "Add function to save data as "*.pkl.gz"
# Assuming your personal fork is called `origin` and you work on local branch `SaveFeature`
git push -f origin SaveFeature:SaveFeature

What is wrong with multiple commits in a PR? I do not think this is really required plus it is a wastage of my time. Each commit happened in a different file and is small and clear enough to follow.

There is nothing wrong with multiple commits. However, since the four commits implement a single functionality, it seems natural to me to commit them as a single commit. A commit makes sense if you may want to revert to it. Only the last is functional, so the others are obsolete in my opinion.
I don't see it as a waste of time, because you could have committed each commit as an amendment to the previous one which would even be faster, because you would not even need to write a commit message.

Frequent and smaller commits are always preferable than a single monolithic commit spanning multiple files. I disagree that only the last commit here was functional; each of these are necessary and not obsolete and just the last commit by itself wouldn't have worked. These are not even amendments as each serves different purpose. We may have different working style but that doesn't mean that this approach is incorrect and I should change this. The code is doing what it is supposed to do and scrutinizing that is where we should be putting our energies on.

I See your point, and, again, I am not saying it is wrong. By amendment I mean it in Git terminology, not that a line of code was changed. I agree that frequent commits are useful. I often make dozens of commits until I complete the PR. However, these commits are not all to stay. Eventually I organize them in digestible packages, often a single commit. Consider this commit history:

Add function to compute mean
Add option for harmonic mean
Fix typo in help text
Catch error if input is not numpy array

While it makes sense to have these commits during development, I don't see a reason to have all of them in the PR, especially the Fix typo in help text. That fix should be included in Add function to compute mean. Would you agree that at least Fix typo in help text is obsolete?

Note, I am fine with keeping four commits. Still I think one is better. It's your call. The code itself is nice and clean.

I think all this is highly dependent on the feature that is being developed and its complexity. If we compile/test locally before making any commit and even before think through the use cases/requirements then things are already good to begin with and one doesn't even need to spend time squashing commits and organizing them. But different people follow different approaches and the version control systems are good at handling these.

The big advantage we get with more commits is during maintenance and debugging. Imagine a scenario where there is a single a commit which has changes in 9 files and each file has 100 lines modified. If the original developer is not reachable for whatever reason and someone else is looking at this for the first time, it is a debugging and maintenance nightmare.

It's difficult to comment on the example you shared without seeing what changes were made in those commits. Was the typo commit fixing something that got introduced in an earlier commit? If the typo commit is fixing many typos across the code base then keeping it separate makes sense.

Thanks for approving the PR. Let's merge this as is and in the future try to keep the number of commits smaller while maintaining clarity.