Skip to content
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

project.mml directory split #3681

Closed
kocio-pl opened this issue Feb 13, 2019 · 13 comments
Closed

project.mml directory split #3681

kocio-pl opened this issue Feb 13, 2019 · 13 comments

Comments

@kocio-pl
Copy link
Collaborator

File project.mml in OSM Carto is very big and hard to manage, currently it has 2639 lines containing 125 KB of data. In German fork it has been lately split into the directory (giggls@a5e0996) with each data layer in a separate file, like project.mml.d/landcover, with Python scripts for composing and decomposing resulting MML (YAML) file:

https://github.com/giggls/openstreetmap-carto-de/tree/master/project.mml.d

What do you think about doing it also in this repo? The problem is that we will loose clear YAML code, which also means additional step during development, on the other hand it makes code maintainability, learning curve and custom changes much easier.

@kocio-pl kocio-pl added the code label Feb 13, 2019
@kocio-pl kocio-pl added this to the Bugs and improvements milestone Feb 13, 2019
@kocio-pl
Copy link
Collaborator Author

BTW: @Adamant36 do you still think about splitting shops.mss file?

@Adamant36
Copy link
Contributor

Yeah. I still think it would be worth doing. I tweaked around with it a little when I was doing the sports PR, but Kosmtik didn't like it for some reason. I'll probably get back to it at some point when I have the time though. Id like to see what @jeisenbe suggests for the 2019 road map and see if I can fit it in there somewhere.

In the mean time, I like how the German fork split the project file. We should totally implement it. it would obviously make things a lot easier. Except the YAML thing, but it seems like a fair sacrifice for the benefits.

@da1910
Copy link

da1910 commented Feb 16, 2019

Separating shops out does make sense I think, taking a look about 500 lines are obviously shops, but theres a lot of features that could be shops, is there some source for what would constitute a shop and an amenity?

@Adamant36
Copy link
Contributor

@da1910, its nice to see someone else supports it. Although I can't think of any examples right now, there's always some overlap between the different tag categories. They aren't really solid definitions. Especially when its comes to amenities. There's been more then one discussion here what constitutes an amenity or not and I don't think we ever came to a final conclusion on it. So it would probably depend. I guess the shop.mss file could contain some things that overlap with amenities. Why not? It would probably have to be figured out before hand though.

@da1910
Copy link

da1910 commented Feb 17, 2019

Moving just the shop category makes shops.mss about 500 lines and amenity-points.mss about 3000. I'm tempted, based on the definition of the amenity category:

Covering an assortment of community facilities including toilets, telephones, banks, pharmacies and schools

To suggest that we move amenities that most likely require payment over to the shops.mss file, though I'm aware that arguably it's no longer just shops. That would include things like bowling alleys, fitness centres (not stations), nightclubs and transportation rental.

@Adamant36
Copy link
Contributor

Adamant36 commented Feb 17, 2019

We could go that way. It wouldn't really matter that much if we. There's zero consequences from moving things out of the amenity file and a lot of upside. I'm pretty sure we create more .mss files for other tags like man made stuff also. Not just shops. Its just a matter of making sure they are included in the project file. You have to test it in Kosmtik when its done also to make sure it loads. I couldn't get it to for some reason. Which is I haven't done it myself.

@kocio-pl
Copy link
Collaborator Author

I would move all the objects we render in a shop style (violet), so not all shop=* tags, if they are rendered as amenity for example.

@da1910
Copy link

da1910 commented Feb 18, 2019

So thinking a little more about this, I'd suggest splitting into three - Amenities are now points that are likely free, and are either manmade or natural, Shops are now points that most likely require payment. There's plenty of edge cases here, but that makes more sense to me than splitting up by icon colour?

(I think the only shop tagged amenity that's not shop coloured is massage at the moment.)

@StyXman
Copy link
Contributor

StyXman commented Feb 18, 2019

The problem is that we will loose clear YAML code

Could you elaborate on that? Programmatically we can make every file a valid YAML file.

@pnorman
Copy link
Collaborator

pnorman commented Feb 22, 2019

File project.mml in OSM Carto is very big and hard to manage, currently it has 2639 lines containing 125 KB of data.

The problem is the large amount of code, not how it's organized.

In German fork it has been lately split into the directory (giggls@a5e0996) with each data layer in a separate file, like project.mml.d/landcover, with Python scripts for composing and decomposing resulting MML (YAML) file:

👎

This adds a significant barrier to entry as it won't work with Kosmtik by itself. We had that situation when we used YAML and there was only JSON support, and it was a hassle then.

@imagico
Copy link
Collaborator

imagico commented Feb 22, 2019

I also think the layer ordering, which is one of the most fundamental things of the style and one of the things that new contributors frequently struggle with, would be fairly non-intuitive by being defined in a separate file (based on file names which are manually selected - which is prone to confusion as well).

IMO if the size of project.mml is considered to be an issue (i am not sure it is but that you can have different opinions on) the clean approach would be working towards including support in CartoCSS and Kosmtik to allow referencing external SQL scripts instead of inlining SQL code. This would work without additional scripts and it would have the advantage of separating the different languages (YAML and SQL).

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Feb 22, 2019

Referencing external SQL in CartoCSS + Kosmtik sounds like the best way for me. Unfortunately CartoCSS is in a limited support state and Kosmtik is not active in last 6 months (we're waiting for kosmtik/kosmtik#295 currently), which makes it not too probable.

@matthijsmelissen
Copy link
Collaborator

👎 per @pnorman and @imagico.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants