search

PreTeXtBook / pretext

PreTeXt: an authoring and publishing system for scholarly documents
https://pretextbook.org
Other
254 stars 203 forks source link

Webwork copy #1445

Closed Alex-Jordan closed 3 years ago

Alex-Jordan commented 3 years ago

This makes it so you can use <webwork copy="some-xml:id-on-a-webwork-element"/>.

The target has to be a PTX-authored WW problem, and there are tests in the generic warnings section of -common about this.

The copying happens during assembly, keeping attributes except the @copy attribute. It copies "all" attributes from the target, except the target's @seed and @xml:id. Well, there aren't any other attributes that the target would have, so this is vacuous, but there in case future attributes come into existence. And then it copies children etc.

Then extract-pg.xsl does its thing on the enhanced source. There is one more python dictionary called copiedfrom. For each webwork that originally had a @copy, there is an entry in copiedfrom for that problem, with value being the @xml:id of the target.

You don't see a change in HTML or PDF output.

When using pretext-ww-problem-sets.xl, no .pg file is built for a problem that is a copy. And in a set definition file, where a copied problem occurs, the file path for the problem is the file path for the "copiedfrom" problem file.

I realize just now I did not put @copy in the schema. I can revisit that. I'm unsure though how you prefer I do it, since you have to regenerate all the downstream schema files. If you like, I can manually edit them all, since it is trivial to visit each of those files and see where legal webwork attributes are declared.

In a separate commit, I added documentation and one example in the sample chapter. Maybe you'd like to put the two commits together? I thought it might help review to separate them.

I tested with a small part of ORCCA, and it works there too.

Alex-Jordan commented 3 years ago

I remembered there was an issue about this: #404

Alex-Jordan commented 3 years ago

I hit an issue with this.

The generic-warnings in -common use $document-root. But when there is a stylesheet that imports both -common and -assembly (in that order), then the $document-root from -assembly is the one this refers to (even though it is a template form -common). So its the $document-root that has assembled source. For some warnings, this wouldn't matter, but for webwork-related warnings, it could. We want to warn the author about things they wrote in their webwork elements, not about whatever may be present in the webwork-reps.

One solution is to use $original instead of $document-root in the warning template from -common. But that will only be valid if -assembly is being used, because -common does not define$original`. There could be some stylesheet that imports -common but not -assembly.

So could I add a variable to -common called $original that is defined identically as in -assembly? And any time -assembly is being used, the assembly version of $original is the one that matters.

Alex-Jordan commented 3 years ago

BTW, this is an issue with the PR because the warning about the target of a @copy not existing is getting triggered when it should not be triggered, simply because the webwork have been replaced with webwork-reps.

Alex-Jordan commented 3 years ago

OK, I found an alternate solution. I think what I've brought up two posts ago may be a larger issue to pursue separately. For the sake of this PR, I'm about to push a commit that reasonably sidesteps the issue.

Alex-Jordan commented 3 years ago

On a separate commit, I put @copy into the schema for convenience. I imagine you may not want this in the schema yet, and the separate commit should make it easy to leave it out.

Alex-Jordan commented 3 years ago

A thought: are we moving away from authors writing an @xml:id? And instead they will write @name? I could change this so that @copy points to an @name to get ahead of that change...

Thinking about this makes me realize I added webwork/@copy to the schema, but forgot to add webwork/@xml:id.

12% reduction in PTX source lines for the first section of ORCCA, following use of @copy. Deleting large chunks is fun.

rbeezer commented 3 years ago

Sorry to be silent. I've been battling a mysterious @xml:id problem all week with no forward progress, so I had to make some headway on some simple stuff. Almost done.

rbeezer commented 3 years ago

Thanks for a comprehensive job on this one. Only enough horesepower for a visual scan tonight. Two busy days with exams coming up, then a mini-break. So a brief pause likely.

Alex-Jordan commented 3 years ago

I'll update things tomorrow, most likely. I think I'll add commits, unless you;d prefer a force push.

Another thing you should probably look closely at is the $this device in -common for the warnings when you @copy to something bad.

Alex-Jordan commented 3 years ago

Here's another question. An author can mess up their @copy in two ways. They might use an xml:id that does not exist. Or they might use an xml:id that points to a webwork but they are pointing to a webwork that uses @source and the problem file lives on the server.

What is the right place to tell the author about this? I put it in the -common general warnings. But it could also be in -assembly right when it is trying to make the copy. That's where I had it at first, but I moved it to -common for reasons I can't remember now.

And then what should happen with processing? I will have it print a dummy exercise explaining what went wrong.

So much is changing, I think it will be nicer to do a force push with clear commits after this. I suggest waiting to look at this again until I've done that force push. Except for that question about where to do the warnings, of course.

Alex-Jordan commented 3 years ago

OK, force-pushed. Now with the generic warnings in -assembly. And if something goes wrong, in addition to the warnings there will be a placeholder problem.

Alex-Jordan commented 3 years ago

Just FYI, cleaning up my local branches included a rebase of this branch off dev. That's what the most recent force push is.

Alex-Jordan commented 3 years ago

The generic "assembly" modal template copies everything from the tree. Now consider an element, say a p, that has an xml:id. With most applications, including the xml:id in the copy is fine and good, because we are really replacing the original with its copy. So we don't double up on an xml:id.

In this application, we really are adding a copy of the webwork. It's easy to drop the webwork's xm:id, but what if buried deeper down, something like a p has an xml:id? I don't want to copy that. This is currently not an issue because nothing deeper inside a webwork should currently have an xml:id. But I could imagine it happening. Say, with an image.

So what do you think if I add a variant to the "assembly" modal templates that I might call "assembly-clean-ids"? It would copy everything, but change an element's xml:id to be something like original-id. Then the assembled source (a) does not have duplicated xml:id and (b) keeps a pointer to the original xml:id, which would be important in some cases, say when there was an xml:id on an image, the resulting image file used that xml:id in its file name, and we want the copy to use the original image file.

This "assembly-clean-ids" would presently only be used within a webwork that is undergoing a copy.

Alex-Jordan commented 3 years ago

I'm already using copied-from at the top of the webwork copy. If you approve of the overall idea here, what's better? copied-from, original-id, or something else?

Alex-Jordan commented 3 years ago

As discussed today in the meeting, I changed things like webwork[xml:id=$copy] to use the id() function. However, I think I can get away with not using xsl:for-each. This is all happening when $b-extracting-pg is true, which only happens when working on original source anyway. So when the context is a webwork that is getting copied, just using id(@copy) without anything more gives me what I want.

Any reason I should explicitly make the context be $original anyway?

I could be wrong about the following. But I would think that if you use for-each to make $original the context, that right then is when some lookup table is built that helps expedite looking up something by its ID. If that is correct, then using the for-each device wouldn't save time, because each time you use it a new lookup table is built. Meanwhile if I can avoid using for-each and just use the actual root tree as context for the id() then it would seem that I get to reuse a lookup table that was built when processing started.

rbeezer commented 3 years ago

I've been battling @xml:id all day, with preliminary work on versions, two directions of partial work on new identifiers (@name), and my own book getting an alternate ending. ;-)

Once all that dust settles, I will probably refactor the whole pre-processor scheme. So I'd prefer no new templates/passes there, and that was as just simple enough to get what you need for now. We can address hypotheticals later? No extra complexity now?

If you are lucky, I won't break your stuff once I refactor. ;-) ;-) ;-)

Alex-Jordan commented 3 years ago

OK, got it. Does it change things if "assembly" mode just has a param for "scrub-ids"? Default is false(), but can pass true() down the tree. Currently implemented in a branch I was playing with.

On Fri, Feb 19, 2021 at 7:59 PM Rob Beezer notifications@github.com wrote:

I've been battling @xml:id all day, with preliminary work on versions, two directions of partial work on new identifiers (@name), and my own book getting an alternate ending. ;-)

Once all that dust settles, I will probably refactor the whole pre-processor scheme. So I'd prefer no new templates/passes there, and that was as just simple enough to get what you need for now. We can address hypotheticals later? No extra complexity now?

If you are lucky, I won't break your stuff once I refactor. ;-) ;-) ;-)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/1445#issuecomment-782554442, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEDOAFWRYJODB2GRXY4QMLS74XSZANCNFSM4XSYOYHQ .

-- Alex Jordan Mathematics Instructor Portland Community College

rbeezer commented 3 years ago

Nice work! Merged but not pushed, since I have some dangerous stuff merged, and want to merge some other dangerous stuff and push all at once. Maybe tonight after dinner. Commit has the number on it. so this should light up once pushed.

You lost the separate schema commit in a force push? I had to hand-edit it out and re-edit to make the other changes go with it. And out of laziness, took full credit. ;-)

rbeezer commented 3 years ago

I thought we agreed that WW problems would not have contents that could be a target of an xref? Which is why copying is "safe" or at least easy?

Maybe we can open this up later, after I refactor the whole @xml:id => @name job? Perhaps there will appear a rational way to support copies (here and elsewhere, e.g sage).

Alex-Jordan commented 3 years ago

Thanks, and I won't begrudge you the schema credit ;) Yeah, I forgot to keep it separate during one of the adjustment force pushes.

I thought we agreed that WW problems would not have contents that could be a target of an xref? Which is why copying is "safe" or at least easy?

Right. Well, before this project "cannot be target of xref" was equivalent to "cannot have an xml:id" (in my mind, anyway). This project and other ID discussions cleared up that the two things are different. Now a "webwork" can have an xml:id, but is still not allowed to be the target of an xref. So maybe that will happen with other things. The one that first comes to mind is an "image", which I now realize as I type this, was already an example: it can have an xml:id, but cannot be target of an xref.

So I think someday there will be things down inside a "webwork" that have an xml:id, even though they should still not be targets of an xref. Not presently an issue in practice, so it can wait. I think the idea of passing a param to the generic "assembly" template is worth looking into when we get there. You'd be saying "from this point down the tree, scrub away unique IDs". Or maybe not really scrubbing them, but storing them in some pointer attribute.

rbeezer commented 3 years ago

When @name becomes a thing, then everything will get an @xml:id from the pre-processor, if it doesn't have an @xml:id already (nee @permid). So @name will be the "issue".

rbeezer commented 3 years ago

Almost forgot. You'll announce webwork/@copy on -announce once I push and once you are confident?

Alex-Jordan commented 3 years ago

OK. Ping me when it is pushed.

On Sat, Feb 20, 2021 at 5:43 PM Rob Beezer notifications@github.com wrote:

Almost forgot. You'll announce webwork/@copy on -announce once I push and once you are confident?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/1445#issuecomment-782779040, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEDOAHBRVSYJHCWOIRE7N3TABQKJANCNFSM4XSYOYHQ .

-- Alex Jordan Mathematics Instructor Portland Community College

rbeezer commented 3 years ago

Pushed.