Accessing non-existent attribute gives warning and other fixes

[mbojan]

This introduces the following updates:

• Added functions has.[vertex/edge/network].attribute(x, attrname) that return FALSE or TRUE if the attribute named attrname is found in network x.
• Functions get.[vertex/edge/network].attribute() give a warning if trying to access a non-existent attribute. The value returned is unchanged, i.e. statnet/network#41
• Updated network.vertex.names() to the new functions. Core behavior unchanged.
• Added missing bipartite attribute to emon networks and resaved them in current serialization format (ver. 3)
• Added tests for the new functionality.

[chad-klumb]

Not a real issue in this case but the pattern here

https://github.com/mbojan/network/blob/795ddfdac05b86f815fe3124ae8f0ccc8245dbde/R/access.R#L1223-L1241

reminded me of this:

https://www.wired.com/2014/02/gotofail/

[mbojan]

Thanks @chad-klumb :D Good catch.

[krivit]

Fixed trailing whitespaces.

[krivit]

Would it be possible to make the new methods generic, for future-proofing sake?

[mbojan]

Would it be possible to make the new methods generic, for future-proofing sake?

Sure. Standby.

[krivit]

Thanks! Since the has.*.attribute.network() methods just call another generic, perhaps make them the .default() methods?

[CarterButts]

Have we seen what this does to our reverse dependencies? It's worth checking what we are breaking - but I think the next release is going to be a large change, in any event, so if we are going to break something, now is probably the time. (All of you know my reluctance to do anything that modifies core behavior for the underlying foundational packages - it almost always leads to tears, often mine.)

On a philosophical note, what this and some other recent changes reflect is a switch in the paradigm for this package. It was originally - and deliberately - written in "permissive" mode, where (1) only minimal requirements were actually enforced, and (2) it was up to the user/developer to ensure that they knew what they were doing. That mode maximizes hackability, and frankly that was the dominant paradigm in the R world at that time (which was why I liked it). Recent years have seen a shift towards a "strict" paradigm, both in statnet and in the R world, in which (1) requirements are enforced, and (2) code/users/developers are not trusted to be able to figure out their own best interests. To some extent, that is probably the inevitable tradeoff towards a world with vast numbers of packages, very deeply stacked toolchains, and a broader user base. Given the ongoing and increasingly complex extensions to network and friends, this may be a direction we'll have to go. (I prefer not to go further than we must, though.)

[mbojan]

@CarterButts , thanks for merging.

Have we seen what this does to our reverse dependencies? It's worth checking what we are breaking - but I think the next release is going to be a large change, in any event, so if we are going to break something, now is probably the time. (All of you know my reluctance to do anything that modifies core behavior for the underlying foundational packages - it almost always leads to tears, often mine.)

I did check 'ergm' and the tests were clean. I do not expect much trouble because I did not change the values returned.

On a philosophical note, what this and some other recent changes reflect is a switch in the paradigm for this package. It was originally - and deliberately - written in "permissive" mode, where (1) only minimal requirements were actually enforced, and (2) it was up to the user/developer to ensure that they knew what they were doing. That mode maximizes hackability, and frankly that was the dominant paradigm in the R world at that time (which was why I liked it). Recent years have seen a shift towards a "strict" paradigm, both in statnet and in the R world, in which (1) requirements are enforced, and (2) code/users/developers are not trusted to be able to figure out their own best interests. To some extent, that is probably the inevitable tradeoff towards a world with vast numbers of packages, very deeply stacked toolchains, and a broader user base. Given the ongoing and increasingly complex extensions to network and friends, this may be a direction we'll have to go. (I prefer not to go further than we must, though.)

I see this trend too. One aspect that I personally find annoying is the fashion of making the packages "opinionated" -- implementing strongly personalized way of solving the problem at hand (not allowing certain types of arguments, hiding useful functions in namespaces etc.). It does ago against the "hacking" you mention. Some R-core members seem to look at things in a similar way when some users go too far, e.g. the reply of Duncan Murdoch in this thread on r-devel. Still, I don't think we go in that direction in this PR. IMHO it is important to make trustworthy tools for the unwilling that will give feedback to the user.

Thanks! Since the has.*.attribute.network() methods just call another generic, perhaps make them the .default() methods?

@krivit , good point about the lack of default methods. However, I think it will be even better to keep methods for networks as they are, but add default methods that will stop() because don't know how to handle class(x). Since this is merged, I'll commit them separately.

[krivit]

@mbojan , that's not quite what I meant. The implementation of has.*.attribute() functions merely calls the corresponding list.*.attributes(), which are all generics. This means that if the list.*.attributes.<CLASS>() is implemented, then has.*.attribute() should work fine for <CLASS>. Thus, it makes sense to call the implementation of has.*.attribute.network() has.*.attribute.default().