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

Sorted definiton.csv #9

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Sorted definiton.csv #9

wants to merge 2 commits into from

Conversation

adityakode
Copy link

@adityakode adityakode commented Apr 1, 2023

organised definition.csv alphabetically
fixes issue #8

@hoijui
Copy link
Owner

hoijui commented Apr 2, 2023

hey Aditya! :-)
thanks!

I have to say though.. I don't know what you did there.
You at least changed the casing of the boolean values two times, without giving a reason, and there might be other changes I don't see. There is also no explanation at all for the first comment .. that it is supposed to achieve.
To sort, we should really only sort, and not change anything else within the same commit. I would do that with a script, and not with any GUI (like LibreOffice), to ensure that such stuff does not happen.
How did you do it?

@adityakode
Copy link
Author

Hi @hoijui . Im extremely sorry for the problems in my commit.
I sorted using script in python, and it did the work , but some rows starting with capital alphabet (License )were at the top .
So I used GUI to put them in their respective order.
If you want the result that came out with only with the script , I would be more than happy to implement it

@adityakode
Copy link
Author

hi @hoijui , this is the result we get using only sort https://github.com/adityakode/osh-dir-std/blob/sort/mod/unixish/definition.csv .
Is this okay ?
PS feel free to comment or point errors if there are any

@hoijui
Copy link
Owner

hoijui commented Apr 3, 2023

This version already has different casing for the boolean values, meaning:
You have stuff like True, False and FALSE, while before everything was true and false only. And given that this changed, I have no confidence in that other stuff did not change as well. I assume the python script parses the CSV, sorts it, and re-serialized it - differently. It would be better to parse, sort, and then write back the original lines in the sorted order, instead of re-serializing the parsed stuff. That would need a different script, and I personally would do it with AWK in this case, though it surely could be done with a modified version of the python-script as well.

Also, there is still the question of how exactly we want to sort; see my comment in Timms original issue.

@hoijui
Copy link
Owner

hoijui commented Apr 3, 2023

Why I care for the casing and only the order of lines changing:
That is generally a good practice in git, to change as little as necessary in one commit. Some code using this CSV might not work anymore if the boolean values have a different casing, which means we would have to change the casing back but keep the new order of lines. also, we would first have to find out why it is not working anymore, and we might find this commit that changed the order of lines.. but why does the parsing fail after that? hmm... (if we might not see the case of boolean values changed right away).

@hoijui
Copy link
Owner

hoijui commented Apr 3, 2023

here is an AWK script that does the job:
https://stackoverflow.com/a/65768883/586229

AS of now, it just sorts alphabetically, so if we wanted to use path-typical sorting with e.g. files first, and maybe ignoring case also, we would have to change that still.

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.

2 participants