[Ellen's Alien Game] Tests are Blocked from Running in Editor & Locally Due to Function Import Error

[lotruheawea]

Dear community,

working on Ellen's Alien Game, the tests do not run and I get this message:

We received the following error when we ran your code:

ImportError while importing test module '.mnt.exercism-iteration.classes_test.py'. Hint: make sure your test modules.packages have valid Python names. Traceback: .usr.local.lib.python3.9.importlib.init.py:127: in import_module return _bootstrap._gcd_import(name[level:], package, level) .mnt.exercism-iteration.classes_test.py:4: in from classes import new_aliens_collection E ImportError: cannot import name 'new_aliens_collection' from 'classes' (.mnt.exercism-iteration.classes.py)

There is from classes import new_aliens_collection in line 4 in classes_test.py. Why is it there?

[github-actions[bot]]

🤖   🤖

Hi! 👋🏽 👋 Welcome to the Exercism Python Repo!

Thank you for opening an issue! 🐍  🌈 ✨

•   If you are requesting support, we will be along shortly to help. (generally within 72 hours, often more quickly).
•   Found a problem with tests, exercises or something else??  🎉
    ◦ We'll take a look as soon as we can & identify what work is needed to fix it. (generally within 72 hours).

​          ◦ If you'd also like to make a PR to fix the issue, please have a quick look at the Pull Requests doc.
             We  💙  PRs that follow our Exercism & Track contributing guidelines!

•   Here because of an obvious (and small set of) spelling, grammar, or punctuation issues with one exercise,
    concept, or Python document?? 🌟 Please feel free to submit a PR, linking to this issue. 🎉

‼️  Please Do Not ‼️ ​        ❗ Run checks on the whole repo & submit a bunch of PRs.               This creates longer review cycles & exhausts reviewers energy & time.               It may also conflict with ongoing changes from other contributors. ​        ❗ Insert only blank lines, make a closing bracket drop to the next line, change a word               to a synonym without obvious reason, or add trailing space that's not an[ EOL][EOL] for the very end of text files.         ❗ Introduce arbitrary changes "just to change things" .         _...These sorts of things are **not** considered helpful, and will likely be closed by reviewers._

• For anything complicated or ambiguous, let's discuss things -- we will likely welcome a PR from you.
• Here to suggest a feature or new exercise?? Hooray! Please keep in mind Chesterton's Fence.
  Thoughtful suggestions will likely result faster & more enthusiastic responses from maintainers.

💛  💙  While you are here... If you decide to help out with other open issues, you have our gratitude 🙌 🙌🏽.
Anything tagged with [help wanted] and without [Claimed] is up for grabs.
Comment on the issue and we will reserve it for you. 🌈 ✨

[lotruheawea]

Ah, the last instruction calls for the creation of function new_aliens_collection. Well, there are two ways, I guess. Either

• create a dummy implementation of new_aliens_collection in the default classes.py or
• use a try-except clause like in lines 6-10.

Having just done the exercise, I think it is good to have the tests running step by step. So the second option would mean they fail with an ImportError...

If you want, I can create a PR implementing the first option:

def new_aliens_collection():
     """

    :param coordinate_pairs: tuple of (x_coordinate: int, y_coordinate: int)
    :return: list of (Alien instances)

    This function should return a `list` of Alien instances,
    each initialized with a single coordinate pair
    """
    pass

[BethanyG]

Hi @lotruheawea 👋🏽

Thanks for filing this issue! There is a bit of a dilemma here, since this exercise was intended to teach both class creation, and some basics around using or calling the class. We also wanted to show that Python supports both defining a class and defining stand-alone functions within the same module. Pre-defining the stand-alone function stub short-circuits that learning somewhat.

But students are getting confused with the wording of task 7:

And, as you pointed out, the import error for new_alien_collection makes it look as though the tests are "broken", unless you are used to decoding Python stack traces.

So yes - the most straightforward "fix" would be to define a stub. What worries me there is that a student then misses out on the "lightbulb" of learning that Python will happily accept functions and classes in the same module, and might also miss out on learning how to interpret stack traces.

We also have an issue where students are not using/reading the stub file, and they erase it, only to have import errors from mis-named classes and functions (hence the first try....except clause).

Soooo....TL;DR I think I am going to ask you to make _both_ changes! 😆

But I would also like to hear your thoughts on how we might ease students into learning how to interpret things like an import stack trace:

• Do we make an exercise around stack traces?
• Do we make an exercise around imports?
• Do we add hints to this exercise (and other exercises) reminding students to check the stack trace/function names?

Not sure that always defining stubs or always trapping import fails is the correct approach for the entire track. It makes sense as students are first learning Python, but I think it might get in the way of fluency as someone progresses. I know the first steps of debugging were a struggle for me - but I am so used to looking at traces now, I barely register the process.

So .. aaaalll that being said - I would happily welcome a PR for this particular exercise, as we work through what our longer-term strategy might be. 😄

[lotruheawea]

Aaaaalright, thank you @BethanyG for your very detailed and descriptive explanation! Maybe I start with my opinion on the strategy. I personally have done some exercises in the Python track a while ago (v. 2), came back now and do the recommended learning exercises. I imagine that this is a common procedures for Python newbies. So eventually, the beginners end up in Ellen's Alien Game and have so far learnt basic Python concepts and types and are familiar with functions. But they know nothing really about scoping or imports or exceptions or stack traces.

Thus, I assume that many could stumble upon this exercise and at the same time have no knowledge about how to interpret the stack trace or are unsure on how to define a function by themselves (just because there were stubs all time). Nevertheless I think, like you, those are important concepts. For this exercise, the topic is OOP and a first introduction to classes. So I would not overload it with the stand-alone function and the interpretation of the stack trace and rather have that in another exercise (but this is only my outsider view).

To your questions:

• Do we make an exercise around stack traces?
  • I think that would be great. Prerequisites could be the classes exercise, an inheritance exercise, an error exercise and then a stack trace exercise? Dunno, if they already exist.
• Do we make an exercise around imports?
  • Again, if that does not exist yet, sure. For me personally, as soon as your code lives in multiple files and tests need to be added (which is always, I guess), imports and code organization become relevant. It took me some time to grasp these concepts and dedicated exercises would be great.
• Do we add hints to this exercise (and other exercises) reminding students to check the stack trace/function names?
  • I would not do this. Especially if we provide hints in the error message (as we do already).

Another compromise for the PR could be splitting the tests on multiple files, one for the Alien class and one for the function. At the same we could use the Alien stub and no new_aliens_collection stub. Alien test cases run and fail and the tests for new_aliens_collection throws an import error (with a hint to implement the function). What do you think?

[PaulT89]

Wait, what's happening? Do I need to do something?

[BethanyG]

@PaulT89 - TL;DR: only if you want to. 😄

I see this as not ... really a bug with the exercise as you've written it, more of an issue with the way we test in general on the track. You see, we make a very large assumption about function names (that they will be there for importing) - but the resulting stack trace can look a heck of a lot like a tooling error, rather than an import error. So, in this particular circumstance, it may be better to "fence" the import in the tests for new_aliens_collection()since making it feels a bit unexpected and some students get confused and think new_aliens_collection() should be a method in the class.

There is a larger discussion here around having a "hanging function" at all, but I am still noodling on whether or not we revise. I am currently leaning toward not revising the task. But maybe there is revision work around explaining why its not in the class? Not sure.

There is also a larger discussion around some exercises that might or might not help students understand import errors, stack traces, etc. But that's longer-term work.

So - in coming back to this exercise after not looking at it a while, if you feel like it needs revising, I am happy to support you in that. But is is FULLY OK for you to take in the discussion, participate (or not) -- and work on something else. 😄

[PaulT89]

Okay, I've got them split them up into classes and classes_standalone_function, though I don't know if the auto-tester is going to choke on the exemplar and exemplar_standalone_function

[BethanyG]

I don't know if the auto-tester is going to choke on the exemplar and exemplar_standalone_function

It will. There is no way to sequentially test solution files. Our test runner assumes one main file under test and it concatenates all tests into one file. In fact, it takes all discovered tests and organizes them by task and line number. So if there is going to be two exemplars, there will need to be two exercises, with their own tests. I think we should probably leave the exercise as it as is and look at how we might explain it better to students.

While I could tell you that I could work on altering the test runner, the fact is that I won't be able to do that quickly, and I wouldn't do it for one exercise.

So I think that fencing in the tests is something we might want to do here - and maybe revising instructions. But separating into multiple files and multiple test runs is ... sorta off the table for the moment.

[bhagerty]

I want to offer an opinion as a learner: I think that it is important to modify the test code so a learner who fails to stub out new_aliens_collection does not get the current error that is raised.

The error currently raised does not give even a smart beginner, who can read some stack traces, a sufficient clue that the problem is in their code. The learner will see this:

ImportError: cannot import name 'new_aliens_collection' from 'classes' (.mnt.exercism-iteration.classes.py)

This tells the learner that a name can't be imported from a file they did not create, i.e., classes.py. No beginner, even a smart one, is going to figure out that the problem is not actually in classes.py, but is in fact in the code they wrote.

You could certainly solve this by providing a different error message—one that clues the learner in to the fact that the problem is not in classes.py but is in their code.

But it would be better to test for the learner's creation of new_aliens_collection separately. The only way I figured out what was going on was by reading this comment thread. Learners should not be expected to do that.

[BethanyG]

@bhagerty @lotruheawea @PaulT89 --

I've submitted PR #3058 to address some of these issues.

The first change is a try-except that "fences" the import. So now the stack track for a missing new_aliens_collection() should read similar to the following:

 ImportError while importing test module '.solution.classes_test.py'.
 Hint: make sure your test modules.packages have valid Python names.
 Traceback:
 solution.classes_test.py:6: in <module>
    from classes import new_aliens_collection
E   ImportError: cannot import name 'new_aliens_collection' from 'classes' (.solution.classes.py)

The above exception was the direct cause of the following exception:
.usr.local.lib.python3.9.importlib.__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level).solution.classes_test.py:8: in <module>
    raise ImportError(\"We tried to import the new_aliens_collection() function from the classes.py file, \"

E   ImportError: We tried to import the new_aliens_collection() function, but could not find it. Did you remember to create it?"

The most important line there being the last one.

---

I've updated the exercise instructions to read:

7. Creating a List of Aliens

Ellen loves what you've done so far, but she has one more favor to ask. She would like a standalone (outside the Alien() class) function that creates a list of Alien() objects, given a list of positions (as tuples).

For example:

>>> alien_start_positions = [(4, 7), (-1, 0)]
>>> aliens = new_aliens_collection(alien_start_positions)
...
>>> for alien in aliens:
        print(alien.x_coordinate, alien.y_coordinate)
(4, 7)
(-1, 0)

---

Finally, I've put a TODO in the stub:

"""Solution to Ellen's Alien Game exercise."""

class Alien:
    """Create an Alien object with location x_coordinate and y_coordinate.

    Attributes
    ----------
    (class)total_aliens_created: int
    x_coordinate: int - Position on the x-axis.
    y_coordinate: int - Position on the y-axis.
    health: int - Amount of health points.

    Methods
    -------
    hit(): Decrement Alien health by one point.
    is_alive(): Return a boolean for if Alien is alive (if health is > 0).
    teleport(new_x_coordinate, new_y_coordinate): Move Alien object to new coordinates.
    collision_detection(other): Implementation TBD.
    """

    pass

#TODO:  create the new_aliens_collection() function below to call your Alien class with a list of coordinates.

---

If you can think of additional behaviors or issues, please comment here or in the PR. Many thanks for your comments and discussion! Once the PR is merged, I will close this issue.