Feat/swc loaderfile Adding SWC loader to load SWC file #384
Conversation
|
Converted to a draft @AdityaBITMESRA. We'll mark it as "ready for review" once the various bits have come in. |
|
|
|
pyneuroml/swc/LoadSWC.py
Outdated
|
|
||
| parts = line.split() | ||
| if len(parts) != 7: | ||
| print(f"Warning: Skipping invalid line: {line}") |
There was a problem hiding this comment.
I'd make this a logger.error or logger.warning perhaps, and we need to decide if this should stop the parsing of the file.
There was a problem hiding this comment.
this still needs to be updated @AdityaBITMESRA
|
Looking good! Notes from the meeting:
|
|
Hi @sanjayankur31 , can you have a look to see if I am going correctly? |
|
Sure, will try to do it today, otherwise it may have to wait for Friday---we have a full day conf/symposium tomorrow so wont' be able to get a lot of time at my desk |
pyneuroml/swc/LoadSWC.py
Outdated
| along the neuronal structure. For more information on the SWC format, see: | ||
| https://swc-specification.readthedocs.io/en/latest/swc.html | ||
|
|
||
| Attributes: |
There was a problem hiding this comment.
These aren't showing up in the docs because they're not in the format we're using. You'll have to use
:param UNDEFINED: ...
:type UNDEFINED: ...
and so on. See the other code bits. The sphinx documentation is here:
https://www.sphinx-doc.org/en/master/usage/domains/python.html#info-field-lists
This is the "default" format, which is what we use.
pyneuroml/swc/LoadSWC.py
Outdated
| except (ValueError, TypeError) as e: | ||
| raise ValueError(f"Invalid data types in SWC line: {e}") | ||
|
|
||
| def to_string(self): |
There was a problem hiding this comment.
We should add a type hint for return val: def to_string(self) -> str: and so on?
pyneuroml/swc/LoadSWC.py
Outdated
| """ | ||
| Add a node to the SWC graph. | ||
|
|
||
| Args: |
There was a problem hiding this comment.
needs to be modified to our docstring format
pyneuroml/swc/LoadSWC.py
Outdated
| except (ValueError, TypeError) as e: | ||
| raise ValueError(f"Invalid data types in SWC line: {e}") | ||
|
|
||
| def to_string(self): |
There was a problem hiding this comment.
Just wondering: what's the difference between adding a new to_string() method vs defining the __str__() method?
https://docs.python.org/3/reference/datamodel.html#object.__str__
Currently, for example, one gets this:
(ins)>>> node = SWCNode(1, 1, 0.0, 0.0, 0.0, 1.0, -1)
(ins)>>> str(node)
'<pyneuroml.swc.LoadSWC.SWCNode object at 0x7f31d89a6810>'
which isn't really informative
| nodes = [] | ||
| for node in self.nodes: | ||
| children = self.get_children(node.id) | ||
| if len(children) > 1 and (type_id is None or node.type == type_id): |
There was a problem hiding this comment.
what would happen if an invalid type id is given?
| Returns: | ||
| list: A list of SWCNode objects that have the specified type ID. | ||
| """ | ||
| return [node for node in self.nodes if node.type == type_id] |
There was a problem hiding this comment.
what happens if an invalid type id is given?
pyneuroml/swc/LoadSWC.py
Outdated
| for type_id in types: | ||
| nodes.extend(self.get_nodes_with_multiple_children(type_id)) | ||
|
|
||
| return nodes |
There was a problem hiding this comment.
we probably want to return a dict with the key being the type and the value being the list of nodes of that type?
The use case here is "give me branch points in axons and dendrites" and we'll probably want to treat them differently. If not, the user can always iterate over all the values.
| return nodes | ||
|
|
||
|
|
||
| def parse_header(line): |
There was a problem hiding this comment.
don't forget the type hints and doc string :)
pyneuroml/swc/LoadSWC.py
Outdated
|
|
||
| parts = line.split() | ||
| if len(parts) != 7: | ||
| print(f"Warning: Skipping invalid line: {line}") |
There was a problem hiding this comment.
this still needs to be updated @AdityaBITMESRA
|
Thankyou for the comments.I will look into it |
|
Hi @sanjayankur31. I have addressed all the comments. Please take a look when time permits |
|
Cool, I'll try to do that today. Could you check and fix the issue ruff is pointing out? (If you set up pre-commit, ruff should run on your system and fix things each time you commit---see the contributing.md file) |
|
I have already setup pre-commit hooks but I think the ruff is pointing out to an error in PlotMorphologyVispy.py file(unused imports) which is not the file I was working on. So what should I do? |
|
Hrm, that must've been fixed in develop. Let me update your branch here on GitHub and that should fix it. |
|
OK, that fixes it. Do remember to pull on your local machine so you get the updates to your branch too. |
|
sure...thankyou |
|
Looks good. I've made some tweaks in #400. Will merge that once the tests pass, and that should merge this one too. |
Review of #384 with a few tweaks
This pull request adds a new module for loading and parsing SWC files