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
[Halpy-136] Add EDDB Diversion Station Loader - [merged] #273
Comments
added 2 commits |
I feel like there has to be a way to simplify this or put it in a loop but I just... can't remember or figure it out. |
In GitLab by @theunkn0wn1 on May 27, 2022, 24:47 Commented on CLI/EDDBFormatter/files/input/.gitignore line 4 reasonably sure git ignores .gitignore files not in the VCS root. |
In GitLab by @theunkn0wn1 on May 27, 2022, 24:47 Commented on CLI/EDDBFormatter/main.py line 21 String concatenation. replace with fstrings |
In GitLab by @theunkn0wn1 on May 27, 2022, 24:47 Commented on CLI/EDDBFormatter/main.py line 36 Consider using pathlib instead. |
In GitLab by @theunkn0wn1 on May 27, 2022, 24:47 Commented on CLI/EDDBFormatter/main.py line 56 Once the file has been read from, you can release the resource. (exit the |
In GitLab by @theunkn0wn1 on May 27, 2022, 24:47 Commented on CLI/EDDBFormatter/main.py line 67
|
In GitLab by @theunkn0wn1 on May 27, 2022, 24:47 Commented on CLI/EDDBFormatter/main.py line 81 for i, key in enumerate(tqdm(... Consider using a dict comprehension here instead. |
In GitLab by @theunkn0wn1 on May 27, 2022, 24:47 Commented on CLI/EDDBFormatter/main.py line 113 Whats the issue here? cattrs can handle dicts... |
In GitLab by @theunkn0wn1 on May 27, 2022, 24:47 Commented on CLI/EDDBFormatter/main.py line 118 like above, |
The problem isn't with cattrs, so much as how we used cattrs later in the bot it expects a list file not a dict file. Not a problem with the code, just what we are expecting and so what we need to create. |
changed this line in version 3 of the diff |
added 1 commit
|
In GitLab by @theunkn0wn1 on May 27, 2022, 24:56 Commented on CLI/EDDBFormatter/main.py line 113 Then why does the comment make it sound like you're working around a issue with cattrs? 🤣 |
Bad comment is bad. #willfix |
changed this line in version 4 of the diff |
changed this line in version 4 of the diff |
changed this line in version 4 of the diff |
changed this line in version 4 of the diff |
added 1 commit
|
changed this line in version 5 of the diff |
added 1 commit
|
changed this line in version 6 of the diff |
added 1 commit
|
I could manage it with the second one but if I take out the first one the program begins throwing a fit. More investigation required. |
changed this line in version 7 of the diff |
added 1 commit
|
added 1 commit
|
os.path does everything we need right now, but if we start working further with the filesystem in the future might be worth looking into. At present, it just doesn't seem really worthwhile for 3 files that use os.path. |
On further review, as the counter does not INCREASE on every loop (only loops that result in qualifying stations) I don't think we can use enumerate here. Thus must remain as is unless I'm missing something. |
resolved all threads |
added 1 commit
|
added 1 commit
|
added 1 commit
|
In GitLab by @theunkn0wn1 on May 28, 2022, 24:33 Commented on CLI/EDDBFormatter/main.py line 101
|
In GitLab by @theunkn0wn1 on May 28, 2022, 24:35 Commented on halpybot/commands/edsm.py line 274 (x) doubt |
In GitLab by @theunkn0wn1 on May 28, 2022, 24:40 Commented on halpybot/commands/edsm.py line 286 shouldn't this be |
changed this line in version 12 of the diff |
added 1 commit
|
added 1 commit
|
Fixed |
resolved all threads |
In GitLab by @rik079 on May 29, 2022, 14:39 approved this merge request |
Merges feature/halpy-136 -> feature/halpy-120c
Adds a system to generate a list of all stations in the galaxy with repair functionality, and uses the new !diversion command to display the 5 closest stations in the galaxy to a client using EDSM lookup coordinates.
EDDB does not have a public API, so this tool is needed to filter down and get the closest stations, very similar to the current DSSA and Landmark systems.
Closes #136
The text was updated successfully, but these errors were encountered: