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

MANT: change morph data naming #37

Closed
chiahaoliu opened this issue Jul 17, 2019 · 5 comments · Fixed by #74
Closed

MANT: change morph data naming #37

chiahaoliu opened this issue Jul 17, 2019 · 5 comments · Fixed by #74
Milestone

Comments

@chiahaoliu
Copy link
Member

Current proposal:

objective -> morphed : data will be manipulated
reference -> target: data will stay unchanged.

The obj-ref language propagates across the repo (like xref, yobj etc), but the convention is rather clear and self-consistent as we always see xref, yref remain unchanged in the code.

The first order fix could be changing the language in pdfmorph function and app since they are user facing.

cc: @sbillinge @pavoljuhas

@pavoljuhas
Copy link
Member

I think we still need both "objective" and "morphed" names. "objective" is the input PDF to be morphed and after transformations becomes "morphed" data.

@sbillinge
Copy link
Contributor

sbillinge commented Jul 17, 2019 via email

@sbillinge sbillinge added this to the 0.1.0 release milestone Nov 17, 2022
@chloeann95
Copy link
Contributor

I believe this gets closed by #46 unless you want me to go through the code and change the code names used throughout as well. Indicating which file is morphed could be done better in the help menu or even in the readout where it gives morphing parameters. Could say something like "X was morphed with" prior to giving the morphing parameters to better remove ambiguity or having to search in the help menu.

@chloeann95
Copy link
Contributor

I think this could be closed by #46 I added some clarity with the documentation where when it mentioned "objective" or "reference" I mark which file input it is but could go farther if we want to changed more thoroughly

@sbillinge
Copy link
Contributor

No, this is not closed by #46. I put comments on that PR to that effect, but I can merge that PR and we can fix this on this PR. Tasks would be:

  1. Look in documentation and change everywhere objective -> morph (data will be manipulated) and `reference -> target) (data will stay unchanged).
  2. Look everywhere in code comments and make the same mapping
  3. Add words to the docs and the README.md that explains the meaning of the words
  4. This is more tricky as getting it wrong will break the code, but change all variables in the code that link to the old naming scheme to reflect the new, so xobjin -> x_morph_in or sthg. The underscores or not depend on making the code as readable and clear as possible.

Doing global searches for the terms in PyCharm and using its "refactor" capabilities should help, but double-check any global copy-replace activities. Being a bit hands-on will help

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 a pull request may close this issue.

4 participants