Fix repetition in bear collection #2594

whonut · 2016-08-08T12:54:35Z

Addresses #2075.

I'm not sure if this is what @sils1297 has in-mind, but it does fix the issue of bears being collected multiple times.

whonut · 2016-08-08T13:10:22Z

I forgot to run the tests...

Adrianzatreanu · 2016-08-09T22:03:49Z

hey you have some partial coverage thingies (the ones with yellow) because the else branches are not being tested. to fix this, you need to test the cases in which the if condition is not met :)

whonut · 2016-08-09T22:06:10Z

I've been trying to sort that since yesterday D: The one thing I can't do is find a GlobalBear to test the bear.kind() conditional with.

sils · 2016-08-10T07:35:50Z

coalib/settings/SectionFilling.py

+        bears_by_section[section_name] = list(section.get("bears", ""))
+
+    bears_to_collect = set(chain.from_iterable(bears_by_section.values()))
+    bear_dirs_to_collect = set(chain(*bear_dirs_by_section.values()))


first you use chain.from_iterable and then chain with *, just use one of them consistenly shall we? ;)

I confess I did this for the sake of line length. Hobgoblin of Little Minds
and all that...

On 10 Aug 2016 08:35, "Lasse Schuirmann" notifications@github.com wrote:

In coalib/settings/SectionFilling.py
#2594 (comment):

for section_name, section in sections.items():

bear_dirs = section.bear_dirs()

bears = list(section.get("bears", ""))

section_local_bears, section_global_bears = collect_bears(

bear_dirs,

bears,

[BEAR_KIND.LOCAL, BEAR_KIND.GLOBAL],

log_printer)

all_bears = copy.deepcopy(section_local_bears)

all_bears.extend(section_global_bears)

fill_section(section, acquire_settings, log_printer, all_bears)

bear_dirs_by_section[section_name] = section.bear_dirs()

bears_by_section[section_name] = list(section.get("bears", ""))

bears_to_collect = set(chain.from_iterable(bears_by_section.values()))

bear_dirs_to_collect = set(chain(*bear_dirs_by_section.values()))

first you use chain.from_iterable and then chain with *, just use one of
them consistenly shall we? ;)

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/coala-analyzer/coala/pull/2594/files/d33f368876807bf83487f02330eeb47dfae37c24#r74198024,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADrZ-M4IaROnb8BzfIoCAZZ673m32sPpks5qeX9YgaJpZM4JfA00
.

Makman2 · 2016-08-17T20:17:56Z

ping :)
@whonut do you need help? :D

whonut · 2016-08-18T11:04:43Z

@Makman2 I'm not sure that I can proceed without some decisions being made about the corner case @sils1297 mentioned above.

AbdealiLoKo · 2016-09-22T09:18:16Z

@whonut I'm not sure what discussions were made before. Could you please elaborate on what corner case @sils1297 mentioned (a link if possible, or a summary of discussions) ?

whonut · 2016-09-22T19:04:10Z

@AbdealiJK Apologies for leaving this half-finished for so long. Life got in the way. I have a few days free at least now. This is the only discussion I can find of it on Gitter with a quick search.

In summary (and as far as I can remember), my change doesn't allow bears with the same class name but defined in different files (e.g. SomeBear in foo/SomeBear.py and SomeBear in bar/SomeBear.py) to be treated separately. As far as I could tell at the time, the release version of coala didn't do this either. It makes some sense as a behaviour though, even if naming different bears identically is to be discouraged.

I hope that's helpful.

AbdealiLoKo · 2016-09-23T02:43:58Z

I think you can ignore two bears with same name but different paths for now. The coala -b flag doesn't support that either to the best of my knowledge. It just chooses one of them (probably randomly)

So make an issue for bears with same name not working correctly in many places, as it would have to be fixed in many places not just this one. (show bears, mentioning bears in cli, mentioning bear as a dependency, etc)

whonut · 2016-09-23T10:18:09Z

@AbdealiJK Will do. I just updated my branch by pulling from upstream, but I just realised that the commit title is overlong. D'oh, not sure what to do about that. Sorry.

I'll fix any CI issues now if I can.

AbdealiLoKo · 2016-09-23T10:32:07Z

Hey, so we don't allow Merge commits like the one you pushed here into master normally. WHat you need to do is rebase on the current master - http://docs.coala.io/en/latest/Developers/Git_Basics.html?highlight=rebase#rebasing

whonut · 2016-09-23T10:35:21Z

OK cool. I did that before, don't know why I forgot. Do I need to do anything before rebasing?

Also, I'm not entirely sure how to fix that failing test.

AbdealiLoKo · 2016-09-24T03:49:22Z

coalib/settings/SectionFilling.py

+        bears_to_collect,
+        [BEAR_KIND.LOCAL, BEAR_KIND.GLOBAL],
+        log_printer)
+    all_bears = copy.deepcopy(all_local_bears)


Why a deepcopy ? Can't we just chain these ?

Honestly I can't remember, chain should work.

It doesn't work properly with just chain, I have to call list on it which makes me think it's something to do with depleting the chain generator. That confuses me though, because I only refer to all_bears once.

gitmate-bot · 2017-02-24T14:25:31Z