search

zoometh / iconr

Formal methods to study Prehistory iconography
GNU General Public License v3.0
11 stars 5 forks source link

Patch 50 #48

Closed josempozo closed 3 years ago

josempozo commented 3 years ago

Improvement in the documentation (list_compar.Rd), including the changes in the code. Especially, the not naming of the vertices by node type.

Small change in the code (list_compar.R and plot_compar.R): The attribute nds.var in the output pairs is now a proper attribute, instead of an additional element in the list.

zoometh commented 3 years ago

To put nds.var as a graph attribute is a good idea, I will check the documentation later

josempozo commented 3 years ago

As it is now is not a graph attribute but a "comparison" attribute. I have assigned it to the list of two graphs (pair) of each comparison. If you want the comparison to be possible on different node variables, it shouldn't be an attribute of the graph itself.

I will continue with the documentation tomorrow. I haven't looked at the vignettes you have completed for the moment. First, I need to complete the help pages.

Best

Jose

Missatge de zoometh notifications@github.com del dia dj., 3 de des. 2020 a les 21:35:

To put nds.var as a graph attribute is a good idea, I will check the documentation later

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zoometh/iconr/pull/48#issuecomment-738291974, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARKCR2NTNAGTEM6NAF4LKRTSS7ZC7ANCNFSM4UMOPZCQ .

zoometh commented 3 years ago

Good

josempozo commented 3 years ago

Hi Thomas,

I have a question on contemp_nds. Why don't we consider the selection of the contemporaneous elements already in a igraph format? We could put the input and output as igraph for the decoration graph. I think that it would be simpler for the user and for the code. Is there any reason for using the dataframes?

Jose

Missatge de zoometh notifications@github.com del dia dj., 3 de des. 2020 a les 23:24:

Good

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zoometh/iconr/pull/48#issuecomment-738372469, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARKCR2PH2BIYGZLGHN7QU7TSTAF2VANCNFSM4UMOPZCQ .

zoometh commented 3 years ago

Hi Jose, I will have a look this WE, re-reading your posts, code changes and documentation. For this latter I will use the same format of description you use for the most completed ones. I'll let you know...

josempozo commented 3 years ago

Hi Thomas,

I'm committing now a change in the labels_shadow documentation and code. There was a bug which has taken me quite some time to solve.

Best

Jose

El dv., 4 de des. 2020, 17:52, zoometh notifications@github.com va escriure:

Hi Jose, I will have a look this WE, re-reading your posts, code changes and documentation. For this latter I will use the same format of description you use for the most completed ones. I'll let you know...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zoometh/iconr/pull/48#issuecomment-738891900, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARKCR2NXAA5U5Z2W2NORCS3STEHVHANCNFSM4UMOPZCQ .

josempozo commented 3 years ago

Sorry. Not in labels_shadow, but in contemp_nds

Missatge de Jose Pozo josmpozo@gmail.com del dia dv., 4 de des. 2020 a les 18:39:

Hi Thomas,

I'm committing now a change in the labels_shadow documentation and code. There was a bug which has taken me quite some time to solve.

Best

Jose

El dv., 4 de des. 2020, 17:52, zoometh notifications@github.com va escriure:

Hi Jose, I will have a look this WE, re-reading your posts, code changes and documentation. For this latter I will use the same format of description you use for the most completed ones. I'll let you know...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zoometh/iconr/pull/48#issuecomment-738891900, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARKCR2NXAA5U5Z2W2NORCS3STEHVHANCNFSM4UMOPZCQ .

zoometh commented 3 years ago

Hi Jose, I'm starting a global reviewing of the package.

Concerning your question about the contemp_nds() function: the user will mostly works with dataframes. He, or she, will not directly use igraph at this stage. This is why, I think, it is good that the function returns a dataframe and not a graph object. For internal purposes, yes, we will maintain igraph data types