Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PG macro library (with latex-image-preamble support) #1478

Merged
merged 4 commits into from
May 24, 2021

Conversation

Alex-Jordan
Copy link
Contributor

Second commit here: small things I noticed should be updated while working on the doc.

Third commit: changes to the debugging messages when building ww-reps.

First commit is the main one.

  • Your document can have a `docinfo/latex-image-preamble[@syntax = 'PGtikz']
  • If it does, then each "webwork" with an image/latex-image[@syntax = 'PGtikz'] gets an extra line that tries to add the latex image preamble content from a macro file.
  • You need to build this macro file using pretex -c pg-macros source.ptx and then upload it to the host course. This must be done before processing the webwork exercises. I put this file for the sample chapter up on the AIM host course already, and also committed it into the sample chapter folder.
  • I don't trust that I edited the schema correctly. But if I did, you will need to build derivative schema files.

@Alex-Jordan
Copy link
Contributor Author

Stand by. Just realized I need to look into where the regular latex-image-preamble is used, and not let the new flavor (`syntax='PGtikz') get in the way.

@Alex-Jordan
Copy link
Contributor Author

OK. I put a new variable in common, $latex-image-preamble. It is the text-sanitized version of what latex-image-preamble has been up to now: one with no @syntax. Then the places that traditionally used latex-image-preamble now use this variable. And one with @syntax='PGtikz' is only used where it is supposed to be used.

Later if something like @syntax='all' is a valid default, it is easy to control that all through the -common variable.

@Alex-Jordan
Copy link
Contributor Author

Got pulled away for life things. I'll holler when this is really ready for you to look at.

@Alex-Jordan
Copy link
Contributor Author

OK, I force pushed after working in the remaining to-dos that occurred to me late. It's still the case that the first commit here is the main one. And ready for your examination. (And will need a schema rebuild once ready.)

@rbeezer
Copy link
Collaborator

rbeezer commented May 21, 2021

First pass looks good. Some nits, as usual. Another meeting before Drop-In so details may need to wait a bit.

@Alex-Jordan
Copy link
Contributor Author

Do you have a stance on <tag>docinfo/latex-image-preamble</tag>? More broadly, putting XPath notation into a <tag>?

@Alex-Jordan
Copy link
Contributor Author

Some nits addressed in a force push, including proper use of <attr>.

@rbeezer
Copy link
Collaborator

rbeezer commented May 22, 2021

Working off previous version, not latest.

  • I'd rather not xref by title, since it is worthless in print. (In offline-mathjax.html) Was there a specific reason for this one?
  • Does Python pg_macros() need the pub_file and stringparams arguments?
  • I'm trying to avoid import-creep and localize uses of import. I think pg_macros() needs import os because I (mistakenly) import it more globally somewhere eles, and I've not cleaned it up yet.
  • I'm also listing just which methods are used due to an import as a comment. Maintaining these lets me see when an import can be removed if no longer needed.
  • In -common: "If they exist, they are amalgamated here" I can't tell who "they" are? Seems just one preamble (singular) is being manipulated, and pronouns are always vague.
  • Thanks for fixing the image-label extraction comments.
  • I was struck by creating a variable once ($latex-image-preamble), and then testing it routinely for emptiness. Rather than testing for the presence of XPath like $docinfo\latex-image-preamble. I think what you have is fine, but we are establishing a pattern for how latex-image-preamble/@syntax will be used later. And what you have done may be the best approach.
  • Someplace you say "In the future..." I try hard to avoid this in the documentation - it ends up sounding dated, or is forgotten, or never happens, or...

Sorry for all the nits - there's a lot going on here. I'll merge if you force-push again.

Let's put schema changes on their own PR from here on out, prefixed with Schema:..... Then I can meld in the derived products without wrecking all the other commits and needing to label them with the PR number. Would that be workable? I can split it out from this one, so don't muck about with this one.

@Alex-Jordan
Copy link
Contributor Author

I'd rather not xref by title, since it is worthless in print. (In offline-mathjax.html) Was there a specific reason for this one?

It points to a description list item, so previously, was getting a PTX warning message when building because it has no number. I was trying to clean up the build. Should I change it back?

That's separate from the real work here, so commenting separately. I'll work on that soon. It's nice if this can be resolved before next week, but not necessary. It's close enough that I would feel good about making edits to ORCCA and APEX based on this even if it's not completely settled.

@rbeezer
Copy link
Collaborator

rbeezer commented May 22, 2021 via email

@Alex-Jordan
Copy link
Contributor Author

Some questions with the first few items here. Then reports of nits cleared.

In -common: "If they exist, they are amalgamated here" I can't tell who "they" are? Seems just one preamble (singular) is being manipulated, and pronouns are always vague.

Will we enforce (in schema) that there is only one docinfo/latex-image-preamble that has no @syntax and/or has @syntax="whatever the default will be"? My XSLT may be wrong here, but I was thinking select="$docinfo/latex-image-preamble[not(@syntax)]" would get all such latex-image-preamble and concatenate them. So if (somehow) an author put more than one in their docinfo, we get them all. I'm all for disallowing them to write more than one, but how to enforce that in schema when we will allow more latex-image-preamble that do have differing values of @syntax?

In any case, I will make the comment more clear if you can give me guidance about those questions I have.

I was struck by creating a variable once ($latex-image-preamble)

Sounds like you are saying this is at least OK for now, but maybe warrants more thought in the long run?


Does Python pg_macros() need the pub_file and stringparams arguments?

Later, once I understand things better, I would like to make it so pub_file stores a location to drop the macro library file. And later, when more things are included in the macros library, I have a suspicion that stringparams may be relevant. I can remove these things for now and bring them back when they get used.

I'm trying to avoid import-creep and localize uses of import. I think pg_macros() needs import os because I (mistakenly) import it more globally somewhere eles, and I've not cleaned it up yet.

If I remove import os right now, it fails. Should I put some sort of to-do cleanup comment for later?

I'm also listing just which methods are used due to an import as a comment. Maintaining these lets me see when an import can be removed if no longer needed.

Got it. Added # join()

Someplace you say "In the future..." I try hard to avoid this in the documentation - it ends up sounding dated, or is forgotten, or never happens, or...

Got it.

Let's put schema changes on their own PR from here on out,

Got it.

@Alex-Jordan
Copy link
Contributor Author

Force pushed. Still up to any changes to make in that comment about having multiple docinfo/latex-image-preamble[not(@syntax)].

@rbeezer
Copy link
Collaborator

rbeezer commented May 23, 2021

I'm all for disallowing them to write more than one

I think the schema could do this, but it'd be messy, and might be the sort of attribute-dependent thing which does not show up in the schema browser after translation. So perhaps not really worth it.

  • You could just concatenate them. Any real harm?
  • Or select the first such with a [1] filter. Authors might figure that out pretty quick. And I don't mind doing this sort of thing. (Tough!)
  • Perfect thing to warn about no matter what in the nascent Schematron replacement. If count(...) > 1, then scream.

Sounds like you are saying this is at least OK for now, but maybe warrants more thought in the long run?

Yes, let's not delay this for this.

I can remove these things for now and bring them back when they get used.

They can stay while you have a plan. Oscar Steven CLI may build on these routines, so stability is good.

Should I put some sort of to-do cleanup comment for later?

If you want. ;-) I know about it, and I know it will cause failure. Just be sure to add the new import in your new routine.

@Alex-Jordan
Copy link
Contributor Author

I changed -common to just grab [1] and added checks to the validation-plus stylesheet.

@rbeezer
Copy link
Collaborator

rbeezer commented May 23, 2021

Ready to go, then? (I still haven't done any testing, but maybe late tonight I'll be in the mood.)

@Alex-Jordan
Copy link
Contributor Author

I think it's ready. At least, it passed my testing and I think all the issues raised in this thread are handled.

rbeezer added a commit that referenced this pull request May 24, 2021
@rbeezer rbeezer merged commit 5a3bb2c into PreTeXtBook:dev May 24, 2021
@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

OK, merged and pushed. Website rebuilding now.

We need a better solution to $refreshCachedImages = 1;, methinks. Test-build of sample-chapter HTML and I got the "special character" graphic instead of the plot of the polynomial with roots (and then the second for-real version right afterwards). Which meshes with my understanding of the cache'ing. Documentation addresses this, so I merged as-is. But I don't think authors (me, too!) need to have this level of understanding (and I can see an FAQ coming).

  • Would using xml:id or visible-id, etc. to name image files address some of this cache'ing and editing mis-match?
  • Or can we devise easy and rational markup to allow authors to avoid this without a deep understanding?

@Alex-Jordan
Copy link
Contributor Author

Just checking something. Did you "manage images" and put the image files on the web server?

Simple test: if you are there and you see the "special character" graph where it should not be, then you "make interactive" does it become the right image?

@Alex-Jordan
Copy link
Contributor Author

At the moment, I cannot get anything at https://pretextbook.org/ to load, so I can't look closely at it. Any reason https://pretextbook.org/ would be down right now?

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

Try again, you might have hit rsync operation.

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

Just checked website HTML version. Neither polynomial nor special character image appears when knowl opens. Problem persists with a page refresh. "Make Interactive" on polynomial will then display plot. Special characters does not have answers, so can't go interactive. Do you see the same?

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

I don't have image management working yet. :-( Send instructions?

Missing images from before was next on my list.

@Alex-Jordan
Copy link
Contributor Author

I was on my school's VPN. Got off, and now things work. Would not be surprised if there is some issue there today that caused trouble with loading pretexbook.org.

What I mean is not new "image management" stuff. I mean old school, where publisher is responsible to move image files to a place where the web page can find them.

If I go to: https://pretextbook.org/examples/webwork/sample-chapter/html/section-9.html
right now, only the last checkpoint has an image. They all have the right image if I make interactive. I wonder if your script that gathers all the image files for upload is only looking for png and missing the svg?

Locally, something similar could have happened where there were image files where they needed to be, but they had not been updated, and images/webwork-60-image-1.svg was still the special characters graph.

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

I'm not certain I know whatt "where they needed to be" means. Talk to me like I'm a 5-year-old. ;-) And I'll work through it.

@Alex-Jordan
Copy link
Contributor Author

If you go here:
https://pretextbook.org/examples/webwork/sample-chapter/html/section-9.html

Then open Checkpoint 9.1, at this point everything is static. Don't hit "Make interactive".

There is no image there (just the alt text description). It is looking for the image on your server at location:
https://pretextbook.org/examples/webwork/sample-chapter/html/images/webwork-58-image-1.svg
But nothing is there. You haven't taken webwork-58-image-1.svg and put it in the images folder on the web server.

Where is webwork-58-image-1.svg locally? I would guess it is in your scratch/webwork-representations/ folder. Also I'm guessing that your build script does go there and get png image files, since the png WW image from checkpoint 9.5 shows up. But at the time you made that script, we only had png images to worry about.

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

OK, thanks. Now second on my list (I'm looking at Python imports for a bit).

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

Post on -dev about schema commits. Just to keep you hopping!

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

Look now.

your build script does go there and get png image files

My build script has evolved into a monster. I drag my feet when it comes to changes. ;-) But that was what I needed - thanks - should be good now. I think we are done with this one now!

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

I think we are done with this one now!

Wrong. ;-) I added the pg-macros component to the help menu. At 4706be6

@rbeezer
Copy link
Collaborator

rbeezer commented May 24, 2021

The import os problem is fixed via 4751361 and at 93af0a6

@rbeezer rbeezer mentioned this pull request May 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants