-
Notifications
You must be signed in to change notification settings - Fork 27
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
Working with a super apex #93
Comments
Just an update here: |
Running
I feel like we had the super-apex as a default at one point, but reverted for some reason. Not sure. In the meantime, what you should do to make progress is to just reconstruct |
In short, we abstracted too much control from the user with the |
Got it. I have another issue laid in here: Would be happy to contribute to fixing up the function (I do think having an if/else type check if whether you're working with a super apex wouuld be relatively straightforward and happy to look into it) |
I like the conversation here and agree with it all (I think). As far as what is feasible to get done (vs what is idea), it might be simplest and easiest for us to take @Lvulis's suggestions and improve the documentation at this time, rather than try to make headway on code changes. What do you think @jonschwenk? |
@Lvulis Can you please upload an MWE so I can debug this? I have plenty of my own examples but it will be easier to work together here if we're working on the same one. I think all I need is your mask (plus the above code you've already provided). |
I think I can knock this out in the coming weeks--it really shouldn't be too heavy of a lift. |
@jonschwenk you can use the mask available via Drive below. I'd be happy to start working on this but it isn't at the top of the list, have been busy with wrapping up paperwork + the move. Are you looking to first document the super apex functionality, or to update the functionality per your original suggestion? https://drive.google.com/file/d/1izX5heHVBlbkJY15gxHHgq850Wp7DUwc/view?usp=sharing |
The (old) script I'm using to extract DCN's is only semi-working to get out a super apex. Leaving a note here to improve documentation and also get a working example to have a super apex.
The output
json
files visualized in QGIS on the Pearl delta, with green circles representing nodes, pink lines representing nodes, and 3 brown circles the inlet nodes. A random node is highlighted (red).Up to this point everything is fine except that delt doesn't contain a super apex node and has the 3 inlet nodes.
Running
compute_topologic_metrics
modifies delt in place and kills all but the widest inlet nodes:Question
add_super_apex
at any point?compute_topologic_metrics
modifies in place, but that only matters for the super apex case?The text was updated successfully, but these errors were encountered: