Skip to content

use [:blank:] character class for SWC parser#3651

Merged
JCGoran merged 7 commits into
masterfrom
swc-parser
Nov 13, 2025
Merged

use [:blank:] character class for SWC parser#3651
JCGoran merged 7 commits into
masterfrom
swc-parser

Conversation

@mgeplf

@mgeplf mgeplf commented Oct 27, 2025

Copy link
Copy Markdown
Collaborator

@mgeplf mgeplf mentioned this pull request Oct 27, 2025
Closed
@azure-pipelines

azure-pipelines Bot commented Oct 27, 2025

Copy link
Copy Markdown

✔️ a8fa0eb -> Azure artifacts URL

mgeplf added 2 commits October 27, 2025 17:26
@mgeplf
use [:blank:] character class for SWC parser
f856642 
* seems to work more reliably on libc++; ie: the default C++ library for macOS
* fixes #3647
@mgeplf
format
d105a7e
@azure-pipelines

azure-pipelines Bot commented Oct 27, 2025

Copy link
Copy Markdown

✔️ d105a7e -> Azure artifacts URL

@codecov

codecov Bot commented Oct 27, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.09%. Comparing base (0f0a6f7) to head (65657b3).
⚠️ Report is 45 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3651      +/-   ##
==========================================
- Coverage   68.09%   68.09%   -0.01%     
==========================================
  Files         686      686              
  Lines      110907   110904       -3     
==========================================
- Hits        75523    75518       -5     
- Misses      35384    35386       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Oct 27, 2025

Copy link
Copy Markdown
Contributor

✔️ d105a7e -> artifacts URL

JCGoran
JCGoran reviewed Oct 28, 2025
View reviewed changes
Comment thread src/ivoc/strfun.cpp
Comment thread src/ivoc/strfun.cpp
Comment thread test/unit_tests/hoc_python/test_swc.py Outdated
Comment thread src/ivoc/strfun.cpp Outdated
@azure-pipelines

azure-pipelines Bot commented Oct 28, 2025

Copy link
Copy Markdown

✔️ bc9a029 -> Azure artifacts URL

@github-actions

github-actions Bot commented Oct 28, 2025

Copy link
Copy Markdown
Contributor

✔️ bc9a029 -> artifacts URL

nrnhines
nrnhines reviewed Oct 28, 2025
View reviewed changes
Comment thread share/lib/hoc/import3d/read_swc.hoc
@github-actions

github-actions Bot commented Nov 3, 2025

Copy link
Copy Markdown
Contributor

✔️ 307ba0a -> artifacts URL

@azure-pipelines

azure-pipelines Bot commented Nov 3, 2025

Copy link
Copy Markdown

✔️ 307ba0a -> Azure artifacts URL

@JCGoran

JCGoran commented Nov 11, 2025

Copy link
Copy Markdown
Collaborator

Can this be merged? Or is it still WIP? Since it's a bugfix it'd be good if the next patch release of NEURON has this change.

@mgeplf

mgeplf commented Nov 11, 2025

Copy link
Copy Markdown
Collaborator Author

Can this be merged? Or is it still WIP? Since it's a bugfix it'd be good if the next patch release of NEURON has this change.

I'm good either way; perhaps the [:blank:] should be a separate fix?
One thing to figure out is if the resultant SWC gives the same results - I don't know how comprehensive the SWC testing suite is.

@azure-pipelines

azure-pipelines Bot commented Nov 11, 2025

Copy link
Copy Markdown

✔️ 19cff35 -> Azure artifacts URL

@mgeplf mgeplf marked this pull request as ready for review November 11, 2025 12:47
@github-actions

github-actions Bot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

✔️ 19cff35 -> artifacts URL

JCGoran
JCGoran reviewed Nov 11, 2025
View reviewed changes
Comment thread external/coding-conventions
@azure-pipelines

azure-pipelines Bot commented Nov 11, 2025

Copy link
Copy Markdown

✔️ 5ed1bb5 -> Azure artifacts URL

@github-actions

github-actions Bot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

✔️ 5ed1bb5 -> artifacts URL

@sonarqubecloud

sonarqubecloud Bot commented Nov 13, 2025

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions

github-actions Bot commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

✔️ 65657b3 -> artifacts URL

@azure-pipelines

azure-pipelines Bot commented Nov 13, 2025

Copy link
Copy Markdown

✔️ 65657b3 -> Azure artifacts URL

@JCGoran

JCGoran commented Nov 13, 2025

Copy link
Copy Markdown
Collaborator

Can this be merged? Or is it still WIP? Since it's a bugfix it'd be good if the next patch release of NEURON has this change.

I'm good either way; perhaps the [:blank:] should be a separate fix? One thing to figure out is if the resultant SWC gives the same results - I don't know how comprehensive the SWC testing suite is.

I think we should take a pragmatic approach:

  • it seems to fix the problem on your end
  • the NEURON CI passes
  • regarding changes to external models, it looks like modelDB CI uses 83 models which contain swc files. I ran the modelDB CI with the branch from this PR for those models, and it looks like there are no differences w.r.t. current release (9.0.0) (see https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/19325819515)
  • NEURON is used by other software, such as netpyne; since netpyne does have some swc files, I ran the tests there as well with this branch, and they pass

Given all of the above, I think we can proceed with merging this PR, and wait to see if the change breaks anything else (there is only so much testing we can do).

@mgeplf

mgeplf commented Nov 13, 2025

Copy link
Copy Markdown
Collaborator Author

@JCGoran > Given all of the above, I think we can proceed with merging this PR

Ok; I don't have access to merge, so I will leave it up to you.

@JCGoran JCGoran merged commit 967bf3a into master Nov 13, 2025
47 checks passed
@JCGoran JCGoran deleted the swc-parser branch November 13, 2025 10:10
JCGoran pushed a commit that referenced this pull request Nov 13, 2025
@mgeplf
use [:blank:] character class for SWC parser (#3651)
53ba371 
* Add test for SWC parser

* Fix #3647
@JCGoran JCGoran mentioned this pull request Nov 13, 2025
Closed
20 tasks
JCGoran pushed a commit that referenced this pull request Nov 13, 2025
@mgeplf
use [:blank:] character class for SWC parser (#3651)
30d9852 
* Add test for SWC parser

* Fix #3647
JCGoran pushed a commit that referenced this pull request Nov 14, 2025
@mgeplf @JCGoran
use [:blank:] character class for SWC parser (#3651)
f7c16a0 
* Add test for SWC parser

* Fix #3647
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JCGoran JCGoran JCGoran left review comments
@nrnhines nrnhines nrnhines approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Import3d_SWC_read in read_swc.hoc doesn't parse comment lines correctly

3 participants

@mgeplf @JCGoran @nrnhines