Purge import hack from network.py

[erlend-aasland]

Always require dependencies to be installed.

Resolves #509

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.86%. Comparing base (3aa509d) to head (de46b7b).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #510      +/-   ##
==========================================
+ Coverage   68.78%   68.86%   +0.07%     
==========================================
  Files          26       26              
  Lines        3117     3112       -5     
  Branches      526      526              
==========================================
- Hits         2144     2143       -1     
+ Misses        835      831       -4     
  Partials      138      138              

Files	Coverage Δ
canopen/network.py	76.96% <100.00%> (+1.67%)	⬆️

[sveinse]

Approved with joy. Thank you @erlend-aasland

[erlend-aasland]

Thanks for the review!

[acolomb]

Sorry that was too quick for me. Would you please leave me some chance to actually review? I haven't grasped at all what this was about and wanted to ask questions. Just too many things at once, and now it moved by too fast...

[sveinse]

Should all PRs run through you @acolomb? If they should, I didn't know about it, I'm sorry. I assumed that two collaborators approving were enough, but I might have been mistaking. Do we have some rules of engagement anywhere in this project so we can avoid running into similar situations?

[acolomb]

No rules that I know of and in general, no I don't insist on approving everything. But in this case I did have some questions. And the other discussions running in parallel took away enough time so I hadn't had a chance to ask them. Just one day to react is a bit short if you're working on this voluntarily, beside $dayjob.

[sveinse]

I understand. And please do not read this as being flippant or self-imposing, but as being rather curious: I count several recently closed PRs that have lifetimes well shorter than 24hrs from opening to merge, so this is not the first. If collaborators have somewhat equal rights then there will be some PR that runs through while not all are available -- even ones where one might have questions. That's a part of running a open source project for us all. So that's why I asked specifically if you are appointed lead and need to explicitly approve PRs.

[acolomb]

The difference lies in the nature of the change. Several other PRs lately had been discussed previously or had little impact and were pretty straightforward or purely additions. This one OTOH has the potential to make the module fail to import when it previously was working. I haven't grasped the justification of the workaround (or the scenario where it's needed) in the first place. But before potentially breaking some user's application with the next update, I'd like to understand fully where this came from and why it's no longer necessary.

And no, "this makes type annotations easier" is not enough of an explanation. :-) The reasoning given in #509 is what I was after.