-
Notifications
You must be signed in to change notification settings - Fork 31
Formatted callsigns csv #661
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
base: main
Are you sure you want to change the base?
Conversation
Pull reviewers statsStats of the last 120 days for UWOrbital:
|
proprogrammer504
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its probably fine, but if u dont mind, could u send an ss into the gs or software general channel and ping @k.wan (thats me)? git doesnt want to load the diffs for this pr, so id like to know exactly what changes were made. based on my understanding of this task, it seems like the task was to validate that callsigns are formatted correctly, and if they are not, correct the formatting.
proprogrammer504
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, the changes should not affect verification, or any code relating to parsing the callsigns. the largest changes was that the all entries are capitalized, and names are cleaned as signs such as periods, or quotation marks were removed.
However, it should be noted that quotations could indicate preferred name / legal name so it may be worth another check.
Syzygicality
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few issues:
- The callsign migration script (run via
python3 gs/backend/migrate.py) no longer works, as a special character is preventing the csv from being parsed by callsigns.py (see line 215 for the breaking character) - The first names column (2nd from the left) still contains middle names, please remove
- Please add double quotation marks to all addresses from the csv
- On closer inspection, there are rgb values for some reason? (see line 247) please resolve
- Enforce single spacing, some fields contains multiple spacing (see line 1064)
- ensure that migrate.py can run by testing callsigns.py
Purpose
Closes #655
Explain the purpose of the PR here if it doesn't match the linked issue. Be sure to add a comment in the linked issue explaining the changes.
New Changes
Identify a proper format for each csv column
Each csv column will only contain all capital letters, numbers (if necessary) and special characters (if necessary).
If there are middle names, they will be included. If there is only an initial that will be included.
any unnecessary characters such as quotations in names have been removed
Clean the data to fit the format
Only modify the csv
Testing
Explain tests that you ran to verify code functionality.
Outstanding Changes
If there are non-critical changes (i.e. additional features) that can be made to this feature in the future, indicate them here.