Remove empty values from the options.

[hansthen]

@Conengmo can you have a look at this? I still need to update four tests because we now remove the None values.

[Conengmo]

Yes this makes a lot of sense. I also think this is the way to go for this. I assume you put the new function in all places that used to haven parse options?

I’ll do a full review tomorrow morning, just wanted to say I think this is a good approach.

[Conengmo]

Great work! Good approach and very thorough. I tried finding instances where we had parse_options before #2029 and are not adding remove_empty now, to see if there's a potential issue. Found only one possible mark:

• I think you missed adding remove_empty in plugins/semicircle.py. That used to have a parse_options call and it seems to have values that are None.

After we get this merged, I'll go ahead and make a v0.19.2 release.

[hansthen]

Great work! Good approach and very thorough. I tried finding instances where we had parse_options before #2029 and are not adding remove_empty now, to see if there's a potential issue. Found only one possible mark:

• I think you missed adding remove_empty in plugins/semicircle.py. That used to have a parse_options call and it seems to have values that are None.

After we get this merged, I'll go ahead and make a v0.19.2 release.

Thanks for the compliments. I felt a tad guilty for breaking the release :-(.

[Conengmo]

I felt a tad guilty for breaking the release :-(.

No worries, those things happen :) I developed a bit of a paranoia about big changes for this reason. But I don't want development to stop because something could go wrong!

That said, a more complete test suite would help here. But doing integration tests in Selenium is much work and quite fragile. Maybe something to think about how we could set that up properly.

[Conengmo]

@hansthen do you want to take a final look and merge? If you can't get to it today, I'll merge this one in a bit. I'll make a release afterwards.

[hansthen]

I felt a tad guilty for breaking the release :-(.

No worries, those things happen :) I developed a bit of a paranoia about big changes for this reason. But I don't want development to stop because something could go wrong!

That said, a more complete test suite would help here. But doing integration tests in Selenium is much work and quite fragile. Maybe something to think about how we could set that up properly.

streamlit-folium uses playwright. I found it quite intuitive. It has a recording mode and a visual playback mode, which makes it easy to debug failing tests. It is not perfect, I found that tests can break randomly due to changes in underlying components, but rather easy to fix.