Skip to content

Conversation

@scarapella
Copy link
Contributor

In my experience, rapids usually occur on waterways that run downhill and/or have tidal effects. So I would propose we only need to check for the rapids tag in those cases.

@jake-low
Copy link
Member

I don't use this feature much myself; for clarity, you're referring to the "Rapids" filter in the upper left dropdown, right?

image

I'm not sure exactly what effect this PR would have on that visualization, but I'm open to merging this if you think it makes the filter more useful. But I can also see an argument that keeping the filter logic simpler means the visual is easier to interpret. So I'd be interested to hear your reasoning for this PR.

@quincylvania
Copy link
Member

Okay yeah, so this is about showing certain features to be implied as complete (teal) instead of unknown (pink) when missing a rapids tag. This pattern is convenient when a tag doesn't really make sense on a feature, but in general I try to be really conservative with implied tags because there are so many edge cases IRL. E.g. there's not reason a waterway=link or waterway=fairway couldn't be drawn through a section of rapids, perhaps to indicate the safe paddle route.

I would err on the side of exclusion instead of inclusion. Any feature with waterway=flowline or tidal=no very likely implies rapids=no, otherwise it's probably a data error.

@scarapella
Copy link
Contributor Author

scarapella commented Oct 14, 2025

Yes, you got the change exactly Quincy. I was (more or less) following the same rationale you had in the code for the labeling of one way (or not) by default i.e. if it goes downhill and isn't tidal, it's one way by default. (Of course those of us that pole upstream ignore that ;-) ).

My rationale was based on similar logic. My feeling is that people use the feature like i do, to check if they might be missing some rapids tag in a place that is likely to have logic. With that in mind I figured the most likely places to have rapids are either those waterways headed downhill (rivers, streams et al.) OR something that IS tidal. I included the tidal exception because i was just about to map the Cobscook Bay Reversing Falls and was also reminded of the Sasanoa Hell Gate, both of which are tidal created rapids.

That being said, rapids are not direction, so I'm happy to reconsider the narrowness of the exclusion/inclusion. Let me have a think on your proposal.

BTW, did you mean (waterway=flowline OR tidal=no) or did you mean (waterway=flowline AND tidal=no)?

@scarapella
Copy link
Contributor Author

I was going to propose that anything that is (waterway=flowline && tidal=no) || (waterway=fairway && tidal=no) implies rapids=no

with the rationale is that

  • waterway=flowline "can be used to map the slow-moving flow of water through a lake, reservoir, or other large water body" ref
  • waterway=fairway should be used "for a linear way representation of a navigable route in a body of water such as a lake or sea, in cases where other values such as waterway=river or waterway=canal are not appropriate." ref

However, I did find several cases where @quincylvania you used a waterway=fairway as a routing around a feature (such as a wing dam) in a river

I could argue for using waterway=link in those cases, but the argument is not a strong one and waterway=fairway is at least as reasonable of a choice.

so when I get a chance i'll re-submit this PR with waterway=flowline && tidal=no) implying rapids=no.

cheers

@scarapella
Copy link
Contributor Author

updated as proposed above.

@jake-low jake-low merged commit 2532eb1 into osmus:main Nov 4, 2025
1 check passed
@jake-low
Copy link
Member

jake-low commented Nov 4, 2025

Sorry for the delay, been busy with other projects. I think this is a good change! Thanks for making the adjustments to the original PR. I've merged this and it will be live on the production site shortly.

@scarapella scarapella deleted the Waterway-Lens-Improvements branch November 5, 2025 15:02
@scarapella
Copy link
Contributor Author

ha, no worries for the timing. It just builds anticipation in the thousands of users of this important feature ;-)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants