kasei / attean

A Perl Semantic Web Framework
19 stars 10 forks source link

Compatibility methods with Trine #113

Closed kjetilk closed 7 years ago

kjetilk commented 7 years ago

I have been working on putting Attean support in RDF::RDFa::Generator, which relies on certain Trine::Node methods. Here's a patch for the methods I found I needed.

I don't know if it is actually a good idea, it is OK to not bring along too much legacy from Trine too. Perhaps the best would be to have a compatibility module, but that would be a lot more work, I would guess.

What do you think?

kasei commented 7 years ago

Depending on the amount of work, I think I would prefer this in a compatibility module. I'd be happy for that module to be included in the Attean package, but I want people to have to opt-in to using the old method names so that we don't accidentally get to a place where there's lots of code using different method names without knowing why.

Do you have any sense of how much more complicated it would be to do this in a separate package instead of using the MooX::Aliases shorthand syntax?

kjetilk commented 7 years ago

Yes, I can absolutely see the argument for that.

So, my main problem with that is that I don't see how to make it simple on the user's side. It could easily be coded as a bunch of roles, but then the user would have to create a class, and instantiate an object, which would make it more painful for them.

Suggestions would be very welcome, though!

kasei commented 7 years ago

Aren't all of the aliases for accessors? Couldn't we just stuff new accessor methods into all of those packages from the new extension file?

kjetilk commented 7 years ago

On Sunday, 13 August 2017 09:32:09 CEST Gregory Todd Williams wrote:

Aren't all of the aliases for accessors?

Most are. There's a predicate and a handler, and for uri, I do the same as for as_string.

Couldn't we just stuff new accessor methods into all of those packages from the new extension file?

Actually, I don't know how you would do that practically... Sub::Install or something?

Kjetil

kasei commented 7 years ago

A TrineCompatability.pm (or something) which has something like:

package Attean::Blank {
  sub blank_identifier {
    my $self = shift;
    return $self->value;
  }
}

Why wouldn't that work?

kjetilk commented 7 years ago

Uhm, dunno. I guess I'll just try and see :-)

kjetilk commented 7 years ago

Can you declare a package more than once...? I couldn't get it to work, running this test:

https://github.com/kjetilk/p5-atteanx-compatibility-trine/blob/f951ee4f2e282589cdf6e5f654f564b8f6673f0f/t/blank.t

I get:

t/blank.t .. 
not ok 1 - use Attean::Blank;
not ok 2 - Attean::Blank->can('blank_identifier')
1..2

#   Failed test 'use Attean::Blank;'
#   at t/blank.t line 6.
#     Tried to use 'Attean::Blank'.
#     Error:  Can't locate Attean/API/Blank.pm in @INC (you may need to install the Attean::API::Blank module) (@INC contains: /mnt/robin/kjetil/dev/p5-atteanx-compatibility-trine/lib /mnt/robin/kjetil/dev/p5-atteanx-compatibility-trine/lib /mnt/robin/kjetil/dev/p5-dist-inkt-profile-tobyink/lib /mnt/robin/kjetil/dev/AtteanX-Store-LDF/lib /mnt/robin/kjetil/dev/RDF-LDF/lib /mnt/robin/kjetil/dev/p5-atteanx-query-cache/lib /mnt/robin/kjetil/dev/p5-atteanx-store-sparql/lib /mnt/robin/kjetil/dev/atteanx-endpoint/lib /mnt/robin/kjetil/dev/attean/lib /mnt/robin/kjetil/dev/p5-rdf-ns-curated/lib /mnt/robin/kjetil/dev/p5-lwp-useragent-chicaching/lib /mnt/robin/kjetil/dev/p5-rdf-trine-store-sparql-cache/lib /mnt/robin/kjetil/dev/URI-NamespaceMap/lib /mnt/robin/kjetil/dev/RDF-Trine-Store-File/lib /mnt/robin/kjetil/dev/RDF-Trine-Node-Literal-XML/lib /mnt/robin/kjetil/dev/RDF-LinkedData/lib /mnt/robin/kjetil/dev/RDF-Helper/lib /mnt/robin/kjetil/dev/RDF-Generator-Void/lib /mnt/robin/kjetil/dev/RDF-Helper-Properties/lib /mnt/robin/kjetil/dev/perlrdf/RDF-Query/lib /mnt/robin/kjetil/dev/perlrdf/RDF-Trine/lib /mnt/robin/kjetil/dev/perlrdf/RDF-Endpoint/lib /mnt/robin/kjetil/dev/Test-RDF/lib /mnt/robin/kjetil/dev/experiments/lib /etc/perl /usr/local/lib/perl/5.18.2 /usr/local/share/perl/5.18.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.18 /usr/share/perl/5.18 /usr/local/lib/site_perl .) at /usr/local/share/perl/5.18.2/Module/Runtime.pm line 317.
# Compilation failed in require at t/blank.t line 6.
# BEGIN failed--compilation aborted at t/blank.t line 6.

#   Failed test 'Attean::Blank->can('blank_identifier')'
#   at t/blank.t line 8.
#     Attean::Blank->can('blank_identifier') failed
# Looks like you failed 2 tests of 2.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/2 subtests 

Test Summary Report
-------------------
t/blank.t (Wstat: 512 Tests: 2 Failed: 2)
  Failed tests:  1-2
  Non-zero exit status: 2
Files=1, Tests=2,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.07 cusr  0.01 csys =  0.09 CPU)
Result: FAIL

Attean is in /mnt/robin/kjetil/dev/attean/lib which is in @INC, so it appears confused. Are you sure it can be done this way?

If it could, wouldn't Sub::Install et al be superfluous?

kjetilk commented 7 years ago

BTW, I was thinking a separate distro is the cleanest way to do it, so I have started that.

kjetilk commented 7 years ago

hmmm, the problem with failure to find it in @INC doesn't appear to have anything to do with the code in the compatibility file... I have to investigate that further...

kjetilk commented 7 years ago

ah, I have to say use Attean first

kjetilk commented 7 years ago

OK; seems I'm getting it to work now! Mildly baffled, but still :-)