Conversation
|
This is a great idea, but I'm concerned that this is one more place to add the atlas (GIN repo, last_versions.conf, docs, readme etc etc), and it will lag behind. Is there a way to directly import this from last_versions.conf somehow? |
|
I looked into this and no it has to be hardcoded like this. Would it be acceptable to automatically create this file using a github action? Thats the only way I think we could have it without hand maintaining it |
Possibly, this seems brittle though. It would be less than ideal, but could it be created on initialising the class? So there would be autocomplete, but only not for the first time someone ever uses the API? |
|
I think its odd to change the python files after everything is installed. Here is what the gh action solution would look like, what do you think? |
|
What about part of the validation script that checks that the list has been updated in the code? |
|
ah so still have it written manually. Well that's already what the test I added does so if someone forgets it will fail when they push. Do you think its beneficial to also do this during atlas validation? |
|
I added it to the atlas validation, I think its good we check twice as they are slightly different (checks atlas is in last_versions.conf vs checking that wrapup atlas name has been added to atlas_name.py) |
tests/atlasapi/test_list_atlases.py
Outdated
| """Ensure atlas name list matches last_versions.conf keys exactly.""" | ||
| atlas_name_values = list(get_args(AtlasName)) | ||
| cache_path = config.get_brainglobe_dir() / "last_versions.conf" | ||
| if not cache_path.exists(): |
There was a problem hiding this comment.
I think we should make sure that our test env always has this set up, so would shouldn't need to check this?
I think we (kind of) already implicitly do because we have a fixture that instantiates a BrainGlobeAtlas that will have been called previously.
There was a problem hiding this comment.
I agree its a bit extra, ill remove it
|
I like the idea a lot! IIUC they just wouldn't have the IDE autocomplete but they could still instantiate the atlas objects? |
|
Yes! I tested it with custom atlases and it works like normal they just don't get the autocomplete |
|
alessandrofelder
left a comment
alessandrofelder
left a comment
There was a problem hiding this comment.
Nice - thanks @PolarBean
Think we can merge now but will leave merging honours to you in case you had more plans for this PR (I self-requested review so wasn't sure).
3 participants
Description
I always find myself needing to check the brainglobe documentation for atlas names. I realised it would be really nice if my IDE could suggest me valid atlas names. This is apparantly very doable and I have implemented it here. The main drawback is we would have a hardcoded atlas name list that we would need to update. We could have a test though that automatically checks for discrepencies between the remote last_versions.conf and our atlas_names.py
This is what it looks like in practice
What is this PR
Checklist: